How to set tag on a transaction?

[jeengbe]

I am genually confused about traces/transactions/spans.
I thought I understood how all goes together (a trace is a collection of transactions, a transaction is a collection of spans, scopes are pushed and popped within a transaction and enrich events generated within them), but recent deprecations have left me wondering.

I want to set a tag on a transaction ("cache.hit"), and it all seems pretty straightforward with Sentry.getActiveTransaction()?.setTag('cache.hit', true);, both methods now deprecated. So Sentry.getActiveSpan()?.setAttribute('cache.hit', true). This, however, ends up settings the tag as an attribute, and only on the very span the tag was set in.

---

Also, unrelated to above, how do I prevent two separate transactions (minutes apart) from using the same trace ID? When a koa request comes in, I run the handler within Sentry.startSpan(() => { ... }), two transactions end up in the same trace. The only top-level "trace" method is Sentry.continueTrace, but that doesn't sound right.

[jeengbe]

Thanks, forgot discussions exist. 😅

[lforst]

If you really want to exclusively set a tag on a transaction you can do the following:

const activeSpan = Sentry.getActiveSpan();
if (activeSpan) {
  Sentry.getRootSpan(activeSpan).setAttribute('foo', 'bar');
}

More easily though, if you're ok with the tag also being set on other events on the same request, you can just do Sentry.setTag('foo', 'bar'). Note that this will require an active isolation scope, which is normally something we create automatically for you with an integration, but in the case of Koa you likely need to do this yourself, but it is pretty straight forward. Just wrap any execution context in Sentry.withIsolationScope(() => { ... }). The right place for this is usually some tracing middleware. I hope that makes sense.

---

As for your second question, that sounds a bit shit. In my humble opinion continueTrace is a bit cursed because it has a stupid side-effect. I will try to get rid of it. What you can do, is:

import { propagationContextFromHeaders } from '@sentry/utils';

withScope(scope => {
  scope.setPropagationContext(propagationContextFromHeaders({sentryTrace: ..., baggage: ..}))
  startSpan(...)
})

Does that sort of help? We are aiming to fix all of this in the next major version btw. We fully acknowledge that the current api is crap.

[jeengbe]

Thanks for the answer!

Unfortunately, no Sentry.getRootSpan, @sentry/node@7.106.0.

Also, this Sentry setup is getting out of hand, did I place the isolation scope correctly, at least? :)

await Sentry.startSpan(
  {
    // Will be refined when _matchedRoute is set
    name: ctx.path,
    op: 'http.server',
    // ms -> s
    startTime: ctx.startTime / 1e3,
    ...traceparentData,
  },
  async (span) => {
    await Sentry.withIsolationScope(async () => {
      // We want to set the Sentry route name _after_ route matching
      // has happened (can't do it before :)), but _before_ the actual route handler
      // is called (or it wouldn't be set when e.g. metrics are logged).
      // By defining a setter for the property, we get the opportunity to inform Sentry
      // of the route before anything else happens.
      // Koa router provides no global hooks, so this is the best we can do.
      if (span) {
        Object.defineProperty(ctx, '_matchedRoute', {
          set(value: string) {
            span.updateName(`${ctx.method} ${value}`);

            // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment, @typescript-eslint/no-unsafe-member-access
            this.__matchedRoute = value;
          },
          get() {
            // eslint-disable-next-line @typescript-eslint/no-unsafe-return, @typescript-eslint/no-unsafe-member-access
            return this.__matchedRoute;
          },
        });
      }

      Sentry.getCurrentScope().setSDKProcessingMetadata({
        request: ctx.request,
      });

      try {
        try {
          await next();
        } finally {
          // Set response status on span before any error handlers etc. run
          if (span) {
            Sentry.setHttpStatus(span, ctx.status);
          }
        }
      } catch (err) {
        // Emit the error instead of throwing it to the next middleware
        // so that error handlers are run within the Sentry async context
        // and transaction metadata is available.
        app.emit('error', err);
      } finally {
        Sentry.metrics.increment('http.server.request', 1);
        Sentry.metrics.distribution(
          'http.server.response_time',
          ctx.deltaTime as number,
          {
            unit: 'ms',
          },
        );
      }
    });
  },
);

[jeengbe]

In regard to the trace issue, unfortunately, the problem is not the baggage. I'm sending fresh requests from Postman, and all requests handled by the same process end up in the same trace. Somewhere, the trace id must leak to a global variable.

[jeengbe]

I'd be happy to wait for v8 if that fixes most of the issues.

[lforst]

Wait, you're right actually. You're not supposed to use getRootSpan(). The only sensical way to do this in the current version (in v7, when using the new performance API) is to just call Sentry.setTag() while an isolation scope is set up. (Or you set a tag on the scope that surrounds the startSpan call that creates the transaction.)

Sorry about the confusion. The new API also trips me up sometimes.

---

Also, this Sentry setup is getting out of hand

Welcome to OpenTelemetry :this_is_fine.jpeg: FWIW I also still have to get used to it and am on the fence about a few things. But: Break things to make things.

did I place the isolation scope correctly, at least? :)

Generally, looks good except for one tiny detail - you likely want the isolation scope to surround the startSpan call. Like so:

await Sentry.withIsolationScope(async () => {
  await Sentry.startSpan(
    {
      // Will be refined when _matchedRoute is set
      name: ctx.path,
      op: 'http.server',
      // ms -> s
      startTime: ctx.startTime / 1e3,
      ...traceparentData,
    },
    async span => { 
      //...
    },
  );
});

That should hopefully also eliminate the bleeding between the requests 🤞

Another nit, when you do Sentry.getCurrentScope().setSDKProcessingMetadata() you likely want to do Sentry.getIsolationScope().setSDKProcessingMetadata() instead, but it shouldn't change that much.

Honestly, I commend the efforts that you're putting into this. You are basically doing what we do whenever we build a new SDK or new instrumentation. Hopefully, not that many people will actually have to use the API like you're doing, but please, if you have feedback, let us know!

[jeengbe]

@lforst Based on your above comment, does that mean that a transaction is not a collection of spans, but instead, the root span itself?

[lforst]

Well, technically it is both, but contractually we only guarantee the latter. Product- and payload-wise a transaction is still a container for a set of spans, however, we are removing the concept of transactions from the SDK and in the product, we will also move into a world of only having spans. Right now, to bridge the gap where the product doesn't yet support ingesting only spans we send "transaction events" as a container for the spans the SDK generates.

All of this is btw to move into a world that is fully OTEL compatible. Eliminating vendor lock-in and such and also allowing us to make use of otel instrumentation (like for example with Koa lol).

[jeengbe]

Lastly, how does Sentry.getCurrentScope().setTag('cache.hit', false); relate to any of the above? It doesn't seem to set the tag at all

[lforst]

Hm, where are you calling this? It depends a bit where you call it. Generally, moving forward, it almost always recommended to just call the top level methods like Sentry.setTag().

[jeengbe]

I don't remember. Drop this comment 🙂