Skip to content

setting TraceID and SpanID fields in Exemplar objects in spanmetricsp… #13401

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Oct 13, 2022
Merged

setting TraceID and SpanID fields in Exemplar objects in spanmetricsp… #13401

merged 1 commit into from
Oct 13, 2022

Conversation

arun-shopify
Copy link
Contributor

@arun-shopify arun-shopify commented Aug 17, 2022 •
edited
Loading

Currently TraceID and SpanID fields of Examplar are set in FilteredAttributes (here) in spanmetricsprocessor and passed around but these fields are not part of FilteredAttributes as per the data model: https://pkg.go.dev/go.opentelemetry.io/collector/pdata/internal/data/protogen/metrics/v1#Exemplar

  • This PR sets TraceID and SpanID fields in Exemplar and also adds SpanID to spanmetricsprocessor
  • prometheusexporter is updated to use TraceID, SpanID fields in Exemplar.

Testing:
Unit tests are updated to test TraceID and SpanID fields instead of checking for those in FilteredAttributes

@arun-shopify arun-shopify marked this pull request as ready for review August 17, 2022 18:02
@arun-shopify arun-shopify requested review from a team and djaglowski August 17, 2022 18:02
@arun-shopify arun-shopify mentioned this pull request Aug 17, 2022
Merged
@dashpole dashpole added processor/spanmetrics Span Metrics processor bug Something isn't working labels Aug 17, 2022
@arun-shopify
Copy link
Contributor Author

Do we need to create a new issue for this so that it can be added to unreleased yaml? I couldn't find an existing issue that states this exact problem but there are a few exemplar related issues that are generally related, not sure if I could use on of those. Or would this be a "chore" item that can skip the change log check.

bogdandrutu
bogdandrutu reviewed Aug 19, 2022
View reviewed changes
@bogdandrutu
Copy link
Member

If no issue, you can use the PR number as the issue number in the unreleased template.

@arun-shopify arun-shopify force-pushed the spanmetrics_exemplars_type_fields branch from 31a6c90 to 925b749 Compare August 23, 2022 21:09
@arun-shopify arun-shopify force-pushed the spanmetrics_exemplars_type_fields branch 2 times, most recently from 91e0ce8 to b49111d Compare August 25, 2022 13:17
@arun-shopify
Copy link
Contributor Author

@bogdandrutu pinging for review :)

@arun-shopify arun-shopify force-pushed the spanmetrics_exemplars_type_fields branch 3 times, most recently from 826fa33 to d49c57f Compare September 6, 2022 20:41
@arun-shopify
Copy link
Contributor Author

@bogdandrutu pinging for review :)

@arun-shopify arun-shopify force-pushed the spanmetrics_exemplars_type_fields branch 2 times, most recently from c83be56 to 9754b05 Compare September 9, 2022 20:12
@bogdandrutu
Copy link
Member

I am not one of the owners of the spanmetricsprocessor. @albertteoh please review

albertteoh
albertteoh approved these changes Sep 16, 2022
View reviewed changes
Copy link
Contributor

@albertteoh albertteoh left a comment

Choose a reason for hiding this comment

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

The spanmetrics processor aspect of this PR looks good to me, thanks for the fix @arun-shopify!

@arun-shopify arun-shopify force-pushed the spanmetrics_exemplars_type_fields branch from 9754b05 to bf2ed93 Compare September 16, 2022 16:07