Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

deprecate exporter API in obsreport #8493

Merged
merged 3 commits into from
Sep 20, 2023

Conversation

codeboten
Copy link
Contributor

@codeboten codeboten commented Sep 19, 2023

The functionality has been moved into exporter helper. This is part of #8492

There is no functional changes, just:

  • code moved from obsreport to exporterhelper
  • tests moved
  • updated references to obsreport.Exporter, obsreport.ExporterSettings, obsreport.NewExporter
  • added aliases and temporary methods to replace ^^^
  • removed duplicated Exporter name in new code location:
    • obsreport.Exporter -> exporterhelper.Exporter
    • obsreport.ExporterSettings -> exporterhelper.Settings
    • obsreport.NewExporter -> exporterhelper.New

The functionality has been moved into exporter helper. This is part of

Signed-off-by: Alex Boten <[email protected]>
@codeboten codeboten requested review from a team and djaglowski September 19, 2023 22:45
Signed-off-by: Alex Boten <[email protected]>
@codecov
Copy link

codecov bot commented Sep 19, 2023

Codecov Report

Patch coverage is 95.09% of modified lines.

Files Changed Coverage
obsreport/obsreport_exporter.go 0.00%
exporter/exporterhelper/obsexporter.go 98.06%
exporter/exporterhelper/common.go 100.00%
exporter/exporterhelper/obsreport.go 100.00%

📢 Thoughts on this report? Let us know!.

@codeboten codeboten merged commit b806833 into open-telemetry:main Sep 20, 2023
@codeboten codeboten deleted the codeboten/obsreport-dep branch September 20, 2023 16:32
@github-actions github-actions bot added this to the next release milestone Sep 20, 2023
Comment on lines +20 to +22
- `obsreport.Exporter` -> `exporterhelper.Exporter`
- `obsreport.ExporterSettings` -> `exporterhelper.Settings`
- `obsreport.NewExporter` -> `exporterhelper.New`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the new API naming is confusing. It still provides the functionality for telemetry reporting but the names don't reflect it anymore. Should we move it exporterhelper/obsreport instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See #8499

codeboten pushed a commit that referenced this pull request Sep 20, 2023
These deprecated methods/structs have been moved to processorhelper:
- `obsreport.BuildProcessorCustomMetricName` ->
`processorhelper.BuildCustomMetricName`
  - `obsreport.Processor` -> `processorhelper.ObsReport`
  - `obsreport.ProcessorSettings` -> `processorhelper.ObsReportSettings`
  - `obsreport.NewProcessor` -> `processorhelper.NewObsReport`

Same as the change for the exporter, no functional changes just moving
code over to the
new location.

~Follows
#8493

---------

Signed-off-by: Alex Boten <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants