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

autoexport: Add support for console exporters #4486

Merged

Conversation

codeboten
Copy link
Contributor

This adds support for the standard output exporter for traces and metrics. To remain consistent with other implementations, i used the key console as the identifier for the exporter. This change is related to the specification pull request open-telemetry/opentelemetry-specification#3742

@codeboten codeboten requested a review from pellared as a code owner October 26, 2023 17:40
@codeboten codeboten requested a review from a team October 26, 2023 17:40
@codecov
Copy link

codecov bot commented Oct 28, 2023

Codecov Report

Merging #4486 (00570c2) into main (19346a7) will decrease coverage by 0.1%.
The diff coverage is 66.6%.

Impacted file tree graph

@@           Coverage Diff           @@
##            main   #4486     +/-   ##
=======================================
- Coverage   80.9%   80.9%   -0.1%     
=======================================
  Files        150     150             
  Lines      10304   10313      +9     
=======================================
+ Hits        8339    8345      +6     
- Misses      1825    1827      +2     
- Partials     140     141      +1     
Files Coverage Δ
exporters/autoexport/spans.go 92.0% <100.0%> (+1.0%) ⬆️
exporters/autoexport/metrics.go 79.5% <50.0%> (-2.4%) ⬇️

Copy link
Member

@pellared pellared left a comment

Choose a reason for hiding this comment

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

open-telemetry/opentelemetry-specification#3742: it would be good to make sure there is an agreement on the "name" before merging 😉

exporters/autoexport/metrics.go Outdated Show resolved Hide resolved
exporters/autoexport/spans.go Outdated Show resolved Hide resolved
Copy link
Contributor Author

@codeboten codeboten left a comment

Choose a reason for hiding this comment

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

Thanks for reviewing @pellared, I agree that having a consistent name would be good :) This is at least consistent w/ 3 (python, js, ruby) implementations. Java uses the term logging https://github.com/open-telemetry/opentelemetry-java/blob/main/sdk-extensions/autoconfigure/README.md#logging-exporter which I worry is causing the same confusion as the logging exporter did in the Collector before it was renamed to debug

Copy link
Member

@pellared pellared left a comment

Choose a reason for hiding this comment

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

This is at least consistent w/ 3 (python, js, ruby) implementations.

I suggest calling it out in open-telemetry/opentelemetry-specification#3742

Alex Boten and others added 3 commits November 1, 2023 08:13
This adds support for the standard output exporter for traces and metrics. To remain consistent with other implementations, i used the key `console` as the identifier for the exporter. This change is related to the specification pull request open-telemetry/opentelemetry-specification#3742

Signed-off-by: Alex Boten <[email protected]>
Signed-off-by: Alex Boten <[email protected]>
@codeboten codeboten force-pushed the codeboten/add-console-exporter branch from 24b5797 to df4f253 Compare November 1, 2023 15:15
CHANGELOG.md Outdated Show resolved Hide resolved
@pellared pellared changed the title support console key in trace/metric exporter env var config autoexport: Add support for console exporters Nov 6, 2023
@pellared pellared merged commit 23181f7 into open-telemetry:main Nov 6, 2023
20 of 21 checks passed
@pellared pellared added this to the untracked milestone Nov 8, 2024
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