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

update stdout trace with resource. #558

Merged
merged 4 commits into from
Mar 16, 2020

Conversation

rghetia
Copy link
Contributor

@rghetia rghetia commented Mar 16, 2020

This adds printing resources in the stdout exporter. The fix is not an elegant fix. resource.Resource fields are not exported. So instead of writing custom json.Marshal for entire SpanData I have wrapped it around an internal _SpanData containing original SpanData and the resources converted to a list of Attributes. _SpanData is then printed using standard json.Marshal.

@rghetia
Copy link
Contributor Author

rghetia commented Mar 16, 2020

It might be better to convert Resource to Span Attributes on a copy of SpanData rather than the earlier solution of wrapping SpanData with another internal object. This is also inline with jaeger and zipkin (TBD).

@jmacd , @MrAlias WDYT?

@jmacd
Copy link
Contributor

jmacd commented Mar 16, 2020

Yes, that sounds good.

@MrAlias
Copy link
Contributor

MrAlias commented Mar 16, 2020

Makes sense to merge with the attributes if that is the plan for Zipkin and Jaeger. Wondering about the Resource field in the SpandData, should we keep a copy of the Resource after merging? Seems a bit redundant, but also more accurate than an empty field.

@rghetia
Copy link
Contributor Author

rghetia commented Mar 16, 2020

Makes sense to merge with the attributes if that is the plan for Zipkin and Jaeger. Wondering about the Resource field in the SpandData, should we keep a copy of the Resource after merging? Seems a bit redundant, but also more accurate than an empty field.

I'll remove it.

@rghetia rghetia merged commit 80b720a into open-telemetry:master Mar 16, 2020
@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