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

Transactions are missing configured tags #1163

Closed
1 of 27 tasks
dparrella opened this issue Jan 14, 2021 · 7 comments · Fixed by #1198
Closed
1 of 27 tasks

Transactions are missing configured tags #1163

dparrella opened this issue Jan 14, 2021 · 7 comments · Fixed by #1198
Labels
enhancement New feature or request performance Performance API issues

Comments

@dparrella
Copy link

Platform:

  • Android -> If yes, which Device API (and compileSdkVersion/targetSdkVersion/Build tools) version?
  • Java -> If yes, which Java (and sourceCompatibility/targetCompatibility) version? JDK 11
  • Kotlin -> If yes, which Kotlin (and jvmTarget) version?
  • NDK -> If yes, which NDK/CMake version?
  • React-Native -> If yes, which version?
  • Timber -> If yes, which version?
  • Log4j2 -> If yes, which version?
  • Logback -> If yes, which version?
  • Spring -> If yes, which version?

IDE:

  • Android Studio -> If yes, which version?
  • IntelliJ -> If yes, which version?
  • Other -> If yes, which one?

Build system:

  • Gradle -> If yes, which version?
  • Buck -> If yes, which version?
  • Bazel -> If yes, which version?
  • Maven -> If yes, which version?
  • Other -> If yes, which one?

Android Gradle Plugin:

  • Yes -> If yes, which version?
  • No

Sentry Android Gradle Plugin:

  • Yes -> If yes, which version?
  • No

Proguard/R8:

  • Enabled
  • Disabled

Platform installed with:

  • JCenter
  • Bintray
  • Maven Central
  • Manually

The version of the SDK:
4.0.0-alpha.2


I have the following issue:

I've noticed that all transactions are missing tags, including the environment and release. These are configured using SentryOptions that we pass in to Sentry.init(). These tags appear on all of our Issues.

It also seems like the SentryBeforeSendCallback we configure is not called, so it is not possible to work around the issue.

We are using the logback and Spring integrations but I am able to reproduce even with just manual calls to send a transaction.

Inspecting the code and setting a breakpoint with a debugger, I can see that SentryClient.captureTransaction() has the correctly configured scope, but the transaction has no tags.

https://github.com/getsentry/sentry-java/blob/main/sentry/src/main/java/io/sentry/SentryClient.java#L343-L378

Steps to reproduce:

Initialize Sentry:

SentryOptions options = SentryOptions.from(PropertiesProviderFactory.create());
options.setDsn("your-dsn-here");
options.setEnvironment("test");
options.setRelease("1234");
options.setTracesSampleRate(1.0);
options.setTag("tag", "bag");

Sentry.init(options);

Send a transaction:

final SentryTransaction transaction = Sentry.startTransaction("test transaction");
final ISpan span = Sentry.getSpan();
final Span innerSpan = span.startChild("test", "a test span");
innerSpan.finish();
transaction.finish();

Actual result:

  • Transaction is visible in the UI but it has none of the expected tags (environment, release, "tag").

Expected result:

  • Transaction should have the same tags we would expect to see on Issues as configured via SentryOptions.
  • SentryBeforeSendCallback should be invoked, as SentryTransaction is another SentryEvent
@marandaneto
Copy link
Contributor

@dparrella thanks for testing out Performance for the Java SDK :)

environment and release are set to transactions too, fixed by #1147 already, and hopefully, there'll be a new alpha version today.

If you want to filter out transactions, you could use the TracesSamplerCallback or tracesSampleRate instead of SentryBeforeSendCallback.

About the tags, indeed, we're still discussing which fields should be set to transactions, We want transactions to be cheap so it doesn't make sense to set everything, right.

Which fields would be vital for you? thanks.

@marandaneto marandaneto added enhancement New feature or request performance Performance API issues labels Jan 14, 2021
@marandaneto
Copy link
Contributor

marandaneto commented Jan 14, 2021

@maciejwalkowiak tags is already part of the SentryBaseEvent but it's not copying the tags over in the processTransaction method, was it purposely or an oversight?

Ideally, we'd need to discuss field per field, what makes sense to be copied over, tags does make sense IMO, they are filterable items on the dashboard.

@maciejwalkowiak
Copy link
Contributor

@marandaneto agreed. Since transactions are not a subject of event processing they are also not processed by the MainEventProcessor. I believe the starting point should be specifying what's in the SentryBaseEvent and do most of the processing on this base class.

@dparrella
Copy link
Author

@dparrella thanks for testing out Performance for the Java SDK :)

environment and release are set to transactions too, fixed by #1147 already, and hopefully, there'll be a new alpha version today.

If you want to filter out transactions, you could use the TracesSamplerCallback or tracesSampleRate instead of SentryBeforeSendCallback.

About the tags, indeed, we're still discussing which fields should be set to transactions, We want transactions to be cheap so it doesn't make sense to set everything, right.

Which fields would be vital for you? thanks.

@marandaneto environment and release are the most important, so thanks for getting that fixed! Without those we can't use those drop-downs to filter transactions in the UI ;-). For the sake of consistency, I do think it is reasonable to expect the same tags that we would see on our Issues. Otherwise the filtering/searching does not work the same. So what I'm saying is, tags are important :).

@maciejwalkowiak
Copy link
Contributor

@marandaneto do we consider this issue as critical part for #1151? I can easily add setting tags but perhaps we are going to have field by field guidelines?

@marandaneto
Copy link
Contributor

@maciejwalkowiak I'd say that tags are part of the bare min. because of filtering/searching, but still, we'd need to figure out field by field.

I guess we could definitely do a PR adding tags and sdk, as the sdk would be important for debugging at the beginning, so we know which versions are sending bad/wrong data if any.
Might be the case that sdk is already appended automatically because the envelope header contains it.

What is left now is the request field, which contains useful info too, this is populated automatically when using servlet and spring, right?

If not super complicated, I'd be sure request is also available thru transactions, but tags I'd consider the bare min.

@marandaneto
Copy link
Contributor

@dparrella would you expect the Scope's tags also to be available under transactions?

Scope's tags are those added at runtime via configureScope, thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request performance Performance API issues
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants