Sentry middleware example improvements

[jpwilliams]

Summary

The Sentry middleware example on the docs has a few issues:

• It runs Sentry.init() - most users will already be initializing the client, and initializing it again can cause issues.
• It uses some now-deprecated APIs for transactions, spans, and scopes. The Custom Instrumentation docs list out a few methods, but many of these are deprecated (see MIGRATION.md), especially the Hub() API.

A helpful user has spent time on this and recommended we use scopes, like so:

    return {
      onFunctionRun({ ctx, fn }) {
        return Sentry.withScope((scope) => {
          // Add specific context for the given function run
          scope.setTags({
            "inngest.client.id": client.id,
            "inngest.function.id": fn.id(client.id),
            "inngest.function.name": fn.name,
            "inngest.event": ctx.event.name,
            "inngest.run.id": ctx.runId,
          });
 
          // Start a transaction for this run
          const transaction = Sentry.startTransaction({
.....

This, along with then changing some code to alter transactions and their child spans (which use APIs which are also now deprecated), we should be able to smarten up the example.

Ideally, this results in an @inngest/middleware-sentry package. It's a very popular middleware.

Tasks

• [ ] Report back to the user on Plain when done

[fnimick]

re: "and initializing it again can cause issues" - in particular if you initialize it using a framework-specific sentry package, then initialize here using @sentry/node, it breaks the path transformation used for sourcemaps.

Related: https://github.com/getsentry/sentry-javascript/issues/10089

[fnimick]

Also, my Sentry scope example doesn't actually work, because the scope is popped off the Sentry scope stack as soon as the function returns - it's not actually present at the time that e.g. the returned inner function transformOutput is called.

Ideally the Sentry scope needs to wrap the actual call made at the Inngest library level from end to end, and then individual callbacks from the middleware can use the Sentry import and have the scope automatically apply.

I have a middleware solution using pushScope and popScope, but ideally this would be a native wrapper in the inngest client library using withScope rather than those deprecated functions.

export const sentryMiddleware = new InngestMiddleware({
  name: "Sentry Middleware",
  init({ client }) {
    return {
      onFunctionRun({ ctx, fn }) {
        const scope = Sentry.getCurrentHub().pushScope();
        scope.setTags({
          "inngest.client.id": client.id,
          "inngest.function.id": fn.id(client.id),
          "inngest.function.name": fn.name,
          "inngest.event": ctx.event.name,
          "inngest.run.id": ctx.runId,
        });

        // Start a transaction for this run
        const transaction = Sentry.startTransaction({
          name: "Inngest Function Run",
          op: "run",
          data: ctx.event,
        });

        let memoSpan: Sentry.Span;
        let execSpan: Sentry.Span;

        return {
          beforeMemoization() {
            // Track different spans for memoization and execution
            memoSpan = transaction.startChild({ op: "memoization" });
          },
          afterMemoization() {
            memoSpan.finish();
          },
          beforeExecution() {
            execSpan = transaction.startChild({ op: "execution" });
          },
          afterExecution() {
            execSpan.finish();
          },
          transformOutput({ result, step }) {
            // Capture step output and log errors
            if (result.error) {
              Sentry.withScope((nestedScope) => {
                if (step) {
                  nestedScope.setTags({
                    "inngest.step.name": step.displayName,
                    "inngest.step.op": step.op,
                  });
                }
                Sentry.captureException(result.error, nestedScope);
              });
            }
          },
          async beforeResponse() {
            // Finish the transaction and flush data to Sentry before the
            // request closes
            transaction.finish();
            Sentry.getCurrentHub().popScope();
            await Sentry.flush();
          },
        };
      },
    };
  },
});

[fnimick]

(for anyone who used the above, my transformOutput hook was only capturing errors that occurred in steps, and not those that occurred at the top level of the function. this is now fixed.)

[aon]

Hey everyone. I made some improvements to log more data and remove the deprecatedstartTransaction. Although we still cannot avoid using the pushScope This is the middleware I'm currently using:

import * as Sentry from "@sentry/nextjs"
import { type Span } from "@sentry/nextjs/types/server"
import { type Primitive, type SpanAttributes } from "@sentry/types"
import { type EventPayload, InngestMiddleware } from "inngest"

export const sentryMiddleware = new InngestMiddleware({
  name: "Sentry Middleware",
  init({ client }) {
    return {
      onFunctionRun({ ctx, fn }) {
        const scope = Sentry.getCurrentHub().pushScope()
        const event = parseInngestEventIntoAttributes(ctx.event)

        scope.setTags({
          "inngest.client.id": client.id,
          "inngest.function.id": fn.id(client.id),
          "inngest.function.name": fn.name,
          "inngest.run.id": ctx.runId,
          ...flattenEventIntoPrimitive(event, "inngest.event"),
        })

        // Start a span for this function run
        return Sentry.startSpan(
          {
            name: "Inngest Function Run",
            op: "run",
          },
          () => {
            let memoSpan: Span | undefined
            let execSpan: Span | undefined

            return {
              beforeMemoization() {
                memoSpan = Sentry.startInactiveSpan({
                  name: "Inngest Memoization",
                  op: "memoization",
                })
              },
              afterMemoization() {
                memoSpan?.end()
              },
              beforeExecution() {
                execSpan = Sentry.startInactiveSpan({
                  name: "Inngest Execution",
                  op: "execution",
                })
              },
              afterExecution() {
                execSpan?.end()
              },
              transformOutput({ result, step }) {
                if (Boolean(result.error)) {
                  Sentry.withScope((nestedScope) => {
                    if (step) {
                      nestedScope.setTags({
                        "inngest.step.name": step.name,
                        "inngest.step.op": step.op,
                      })
                    }
                    Sentry.captureException(result.error)
                  })
                }
              },
              async beforeResponse() {
                Sentry.getCurrentHub().popScope()
                await Sentry.flush()
              },
            }
          },
        )
      },
    }
  },
})

function parseInngestEventIntoAttributes(event: EventPayload) {
  const stringifyWithoutFail = (data: unknown) => {
    try {
      return JSON.stringify(data)
    } catch {
      return "Failed to stringify"
    }
  }

  const attributes = {
    name: event.name,
    data: stringifyWithoutFail(event.data),
    ts: event.ts,
    user: stringifyWithoutFail(event.user),
    v: event.v,
  } satisfies SpanAttributes

  return attributes
}

function flattenEventIntoPrimitive(
  event: ReturnType<typeof parseInngestEventIntoAttributes>,
  prefix: string,
) {
  const result: Record<string, Primitive> = {}

  for (const [key, value] of Object.entries(event)) {
    result[`${prefix}.${key}`] = value
  }

  return result
}

[mmachatschek]

@jpwilliams can you verify the above example by @aon and update the middleware example in the docs? sentry-js v8 has been released two weeks ago and it would be nice to have an official example on the website

[brijmcq]

@aon what version of sentry are you using? I tried your code and getting this error and warning:

• pushScope does not exists on type Hub
• getCurrentHub() is already deprecated

[aon]

Hey, I haven't touched much the code since I posted it here but it's still running in a production app. This is the version I'm using:

"@sentry/nextjs": "^7.103.0"

Both getCurrentHub and pushScope are deprecated, but it was the only solution at the time to implement the middleware. Let me know if it works!

[jpwilliams]

Hi folks!

Appreciate the conversation here. It's not a great experience for anyone having to scrape the solution from our docs and, as we've seen, it inevitably goes out of date.

I've added a PR for a new @inngest/middleware-sentry package over at #598. It's on npm but I'd love to get some comments here and/or on that PR to make sure it works for all of you before we ship it proper.

Installation and usage from that PR looks like:

npm install @inngest/middleware-sentry@pr-598

import * as Sentry from "@sentry/node";
import { Inngest } from "inngest";
import { sentryMiddleware } from "@inngest/middleware-sentry";

// Initialize Sentry as usual wherever is appropriate
Sentry.init(...);

const inngest = new Inngest({
  id: "my-app",
  middleware: [sentryMiddleware()],
});

Right now this is v8+ due to the usage of the new APIs.

Options are also detailed in that PR. @aon's parseInngestEventIntoAttributes() functions are interesting, too, and may be reasonable to put in at least behind an option.

Please let me know if this works and we'll get that package shipped. 🙂 Leaving this issue open for discussion and feedback until that's done.

[brijmcq]

@jpwilliams works on my end :) Thank you for responding really quick!

[jpwilliams]

As there has been no further feedback since the release (#638) of @inngest/middleware-sentry (#598), I'm closing this.

Please open another issue referencing that new package for any further requests!