Skip to content

Conversation

loewenheim
Copy link
Contributor

This modifies the {set,remove}_{tag,extra,context} methods on Scope to also modify the current span/transaction in the corresponding manner.

Fixes #587.

@loewenheim loewenheim self-assigned this Jun 9, 2023
@codecov
Copy link

codecov bot commented Jun 9, 2023

Codecov Report

Merging #589 (df66ed7) into master (4b886a8) will decrease coverage by 0.75%.
The diff coverage is 2.56%.

❗ Current head df66ed7 differs from pull request most recent head 3ee3dbd. Consider uploading reports for the commit 3ee3dbd to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #589      +/-   ##
==========================================
- Coverage   70.74%   69.99%   -0.75%     
==========================================
  Files          67       67              
  Lines        6843     6919      +76     
==========================================
+ Hits         4841     4843       +2     
- Misses       2002     2076      +74     

@kamilogorek
Copy link
Contributor

Wouldn't it be possible to perform the same apply_scope, somewhere here? (ofc modified to this need)

// TODO: apply the scope to the transaction, whatever that means

@loewenheim loewenheim requested a review from Swatinem June 12, 2023 08:51
@Swatinem
Copy link
Member

Agreed with @kamilogorek , doing all this at the time when finishing / sending a transaction is the better thing to do, similar to the send_event and apply_scope things that happen for events.

@loewenheim
Copy link
Contributor Author

Wouldn't it be possible to perform the same apply_scope, somewhere here? (ofc modified to this need)

// TODO: apply the scope to the transaction, whatever that means

I don't understand where the scope is supposed to come from in this place. In the case of events, the scope is explicitly passed to capture_event:

pub fn capture_event(&self, event: Event<'static>, scope: Option<&Scope>) -> Uuid {

@Swatinem
Copy link
Member

Ah yeah, because you always call capture_event with the current hub that has the scope. For transactions, you can in theory pass those around independently of the current hub. But really I don’t think that matters much. Just access the current hub whenever you finish the transaction.

@loewenheim
Copy link
Contributor Author

Closed in favor of #590

@loewenheim loewenheim closed this Jun 13, 2023
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.

Make sure tags, user, &c. on scopes are applied to transactions
3 participants