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

stdoutmetric: Add WithWriter and WithPrettyPrint options #4507

Merged

Conversation

dmathieu
Copy link
Member

While stdoutmetric technically allows setting a custom writer with WithEncoder, using it means having to export the json package within an application, while stdouttrace exposes the quite simpler WithWriter method.

This PR therefore introduces WithWriter into stdoutmetric, so a writer other than os.Stdout can be passed easily.
It also introduces WithPrettyPrint, so the package provides the same options as stdouttrace does.

@codecov
Copy link

codecov bot commented Sep 14, 2023

Codecov Report

Merging #4507 (2dfde9e) into main (9bbefc6) will increase coverage by 0.0%.
The diff coverage is 100.0%.

Impacted file tree graph

@@          Coverage Diff          @@
##            main   #4507   +/-   ##
=====================================
  Coverage   81.3%   81.3%           
=====================================
  Files        220     220           
  Lines      17679   17689   +10     
=====================================
+ Hits       14384   14395   +11     
+ Misses      2994    2993    -1     
  Partials     301     301           
Files Coverage Δ
exporters/stdout/stdoutmetric/config.go 92.0% <100.0%> (+4.5%) ⬆️

@dmathieu dmathieu force-pushed the stdoutmetric-writer-prettyprint branch from 8040f1a to 1906f12 Compare September 14, 2023 14:39
@MrAlias MrAlias added the pkg:exporter:stdout Related to the stdout exporter package label Sep 14, 2023
@dmathieu dmathieu force-pushed the stdoutmetric-writer-prettyprint branch from 1906f12 to 0451abd Compare September 18, 2023 15:43
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.

I like that the PR tries to consolidate the two exporter stdout modules 👍

exporters/stdout/stdoutmetric/config.go Outdated Show resolved Hide resolved
exporters/stdout/stdoutmetric/config.go Outdated Show resolved Hide resolved
exporters/stdout/stdoutmetric/config.go Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
@pellared pellared changed the title Add WithWriter and WithPrettyOptions to stdoutmetric stdoutmetric: Add WithWriter and WithPrettyPrint options Sep 22, 2023
@pellared
Copy link
Member

@MadVikingGod PTAL

@dmathieu dmathieu force-pushed the stdoutmetric-writer-prettyprint branch from 820a7c2 to 2dfde9e Compare September 27, 2023 12:47
@MadVikingGod MadVikingGod merged commit 1410496 into open-telemetry:main Sep 27, 2023
24 checks passed
@dmathieu dmathieu deleted the stdoutmetric-writer-prettyprint branch September 27, 2023 15:02
@MrAlias MrAlias added this to the v1.19.0/v0.42.0 milestone Sep 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg:exporter:stdout Related to the stdout exporter package
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

5 participants