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

feat(spans): Extract main_thread tag for spans #2761

Merged
merged 7 commits into from
Nov 27, 2023

Conversation

shruthilayaj
Copy link
Member

Extracts main_thread tag for spans based on span[data][thread.name].
Right now, we only expect to extract this if thread name is main.

#skip-changelog

@shruthilayaj shruthilayaj marked this pull request as ready for review November 23, 2023 16:31
@shruthilayaj shruthilayaj requested review from a team and jjbayer November 23, 2023 16:31
.and_then(|value| value.as_str())
{
if thread_name == MAIN_THREAD_NAME {
span_tags.insert(SpanTagKey::MainTread, "true".to_owned());
Copy link
Member

Choose a reason for hiding this comment

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

Can we limit this to MOBILE_SDKS.contains(...), like the Mobile tag?

Copy link
Member Author

Choose a reason for hiding this comment

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

yup 👍, ae31b46

@shruthilayaj shruthilayaj requested a review from jjbayer November 23, 2023 20:38
@@ -128,6 +132,7 @@ pub(crate) fn extract_span_tags(event: &mut Event, config: &Config) {
// TODO: To prevent differences between metrics and payloads, we should not extract tags here
// when they have already been extracted by a downstream relay.
let shared_tags = extract_shared_tags(event);
let sdk_name = event.sdk_name().to_owned();
Copy link
Member

Choose a reason for hiding this comment

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

nit: It would be slightly cheaper to set let is_mobile = MOBILE_SDKS.contains(event.sdk_name()) here, and plug that into extract_tags.

Or even reuse the value that extract_shared_tags wrote to shared_tags.get(SpanTagKey::Mobile).

Copy link
Member Author

Choose a reason for hiding this comment

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

Co-authored-by: Joris Bayer <joris.bayer@sentry.io>
@shruthilayaj shruthilayaj merged commit 9c571cf into master Nov 27, 2023
@shruthilayaj shruthilayaj deleted the shruthi/feat/extract-main-thread-tag branch November 27, 2023 15:07
jan-auer added a commit that referenced this pull request Nov 29, 2023
* master: (27 commits)
  ref(metric-meta): Add metric for total incoming metric meta (#2784)
  feat(server): Return global config status for downstream requests (#2765)
  ref(processor): Create event processor sub-module with related code (#2779)
  fix(metrics): Temporarily restore previous configuration keys for bucket splitting (#2780)
  feat(metrics): Add source context to code locations (#2781)
  ref(processor): Split off profile processor code into separate sub-module (#2778)
  ref(processor): Split off replay processing code into separate sub-module (#2776)
  ref(metrics): Partition and split metrics buckets just before sending (#2682)
  feat(spans): Allow well-known path segments in resource URLs (#2770)
  ref(processor): Move user and client reports processing into separate submodule (#2772)
  ref(crons): Include message_type in kafka message (#2723)
  ref(spans): Split tag mapping in specific configs (#2773)
  release: 23.11.2
  feat(metric-meta): Normalize invalid metric names (#2769)
  feat(spans): Extract main_thread tag for spans (#2761)
  Add DE Deployments (#2746)
  ref(processor): Move sessions related code into separate sub-module (#2768)
  ref(metric-meta): Capture envelope payload in a sentry issue (#2767)
  release: 0.8.38
  ref(normalization): Restore span processing to transactionprocessor (#2764)
  ...
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.

2 participants