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
@github-actions
Copy link
Contributor

github-actions bot commented Oct 1, 2022

This PR was marked stale due to lack of activity. It will be closed in 14 days.

@github-actions github-actions bot added the Stale label Oct 1, 2022
@arun-shopify arun-shopify force-pushed the spanmetrics_exemplars_type_fields branch from bf2ed93 to 6729fdd Compare October 5, 2022 20:59
@arun-shopify
Copy link
Contributor Author

arun-shopify commented Oct 5, 2022 •
edited
Loading

@bogdandrutu, @albertteoh, pinging for merging if things looks okay.

@github-actions github-actions bot removed the Stale label Oct 6, 2022
@arun-shopify arun-shopify force-pushed the spanmetrics_exemplars_type_fields branch from 6729fdd to ff800e5 Compare October 6, 2022 13:46
@arun-shopify arun-shopify force-pushed the spanmetrics_exemplars_type_fields branch from ff800e5 to 5c38543 Compare October 6, 2022 14:21
@arun-shopify
Copy link
Contributor Author

@Aneurysm9 @djaglowski pinging for review :)

@arun-shopify arun-shopify force-pushed the spanmetrics_exemplars_type_fields branch from 5c38543 to 9f93fa0 Compare October 6, 2022 17:09
@albertteoh
Copy link
Contributor

@bogdandrutu, @albertteoh, pinging for merging if things looks okay.

@arun-shopify I don't have permission to merge. By the way, it looks like there are merge conflicts that need resolving?

@arun-shopify arun-shopify force-pushed the spanmetrics_exemplars_type_fields branch from 9f93fa0 to 50ae063 Compare October 11, 2022 13:55
@arun-shopify
setting TraceID and SpanID fields in Exemplar objects in spanmetricsp…
8a50034 
…rocessor and removing the use of FilteredAttributes to pass these values around.
@arun-shopify arun-shopify force-pushed the spanmetrics_exemplars_type_fields branch from