feat(solidstart): Add server action instrumentation helper #13035

andreiborza · 2024-07-24T11:04:13Z

Can be used like this:

const getUserData = async () => {
  'use server';
  return await withServerActionInstrumentation('getData', () => {
    return { prefecture: 'Kanagawa' };
  });
};

Shows up like this:

Can also be used for api routes like this:

export async function GET() {
    return await withServerActionInstrumentation('getUser', () => {
        return json({ prefecture: 'Akita' })
    })
}

Shows up like this for pageloads on a route that makes a fetch call to an api route

s1gr1d · 2024-07-24T11:42:17Z

packages/solidstart/src/server/utils.ts

+  const sentryTrace = (headers && headers.get('sentry-trace')) || undefined;
+  const baggage = (headers && headers.get('baggage')) || null;


L: is there a reason why one is set to null and one to undefined?

I'm not sure tbh, the types were already like this. I think I could change it both to null but no strong feelings.

Lms24

LGTM! We could think about extracting the getTracePropagationData and flushIfServerless out to utils because we have them in at least 3 SDKs each. But for now it's also fine to live with the duplication.

Lms24 · 2024-07-24T13:22:29Z

packages/solidstart/src/server/utils.ts

+  const hasValidLocation = typeof error.headers.get('location') === 'string';
+  const hasValidStatus = error.status >= 300 && error.status <= 308;
+  return hasValidLocation && hasValidStatus;


out of curiosity/no action required: the status code makes total sense to me. Why is thelocation header important?

I suppose people might set some weird status codes on errors that aren't redirects and we wouldn't want to ignore those? Not sure tbh, no strong feelings on removing this check.

Ah nevermind, I read up on the location header. Makes sense to me!

chargome

LGTM 🚀

Maybe we should also check if this works as expected on vercel/edge

andreiborza · 2024-07-29T12:59:14Z

I rewrote this since the previous push to not use continueTrace and instead just start a span to get proper nesting under the http spans.

Additionally, I only rewrite the transaction name when the target isn't /_server which happens for example for pageloads to a route that uses a server action. That way we keep the more meaningful GET /some-route over GET someServerAction.

mydea · 2024-07-29T13:39:00Z

packages/solidstart/src/server/withServerActionInstrumentation.ts

+ * function body with Sentry Error and Performance instrumentation.
+ */
+export async function withServerActionInstrumentation<A extends (...args: unknown[]) => unknown>(
+  serverActionName: string,


Is there some way to infer the name, possibly? That would be ideal... if not, this is fine for now!

Don't think so. Accessing a parent's function name isn't available in strict mode, also it wouldn't work for minifed code either. Unfortunately I think we have to live with this :(

Lms24 · 2024-07-29T13:37:56Z

dev-packages/e2e-tests/test-applications/solidstart/tests/performance.server.test.ts

+        description: 'getPrefecture',
+        data: {
+          [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'function.server_action',
+          [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'manual',


l/m: I'm not sure if manual is the correct origin here b/c it's not users calling startSpan. However, the way I understand the usage, users have call the wrapper manually, right? So it's probably fine as long as we don't auto instrument server actions.

nothing we emit from the SDK OOTB should have origin: manual - that's the base rule of thumb here :D We should always assign a proper origin that matches (see https://develop.sentry.dev/sdk/performance/trace-origin/) for any span that any SDK starts.

Correct, user's have to manually wrap their server action function bodies with the wrapper.

How about auto.function.solidstart? The sveltekit sdk uses auto.function.sveltekit for wrapServerLoadWithSentry.

Updated, please have another look.

Lms24 · 2024-07-29T13:38:36Z

packages/solidstart/src/server/utils.ts

+ * Takes a request event and extracts traceparent and DSC data
+ * from the `sentry-trace` and `baggage` DSC headers.
+ */
+export function getTracePropagationData(event: RequestEvent | undefined): {


l: do we still need this function?

Ah, good catch! I had it removed and the re-added after trying some things out. Forgot to remove it again :)

Lms24 · 2024-07-29T13:46:43Z

packages/solidstart/src/server/withServerActionInstrumentation.ts

+        op: 'function.server_action',
+        name: serverActionName,
+        attributes: {
+          [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route',


is this a route though? Users could specify anything here, right? I guess component makes more sense?
I tried looking up the transaction name source values but only found this orphaned page. To me the component definition makes sense but feel free to overrule me :D

Judging from that page, component also makes the most sense to me. I initially had it on url but changed it to route after @mydea looked over it.

wdyt about using component instead @mydea?

yeah, component makes sense to me too! Sorry about the misdirection to use route 😅

github-actions · 2024-07-29T16:04:03Z

size-limit report 📦

Path	Size
@sentry/browser	22.45 KB (0%)
@sentry/browser (incl. Tracing)	34.22 KB (0%)
@sentry/browser (incl. Tracing, Replay)	70.26 KB (0%)
@sentry/browser (incl. Tracing, Replay) - with treeshaking flags	63.59 KB (0%)
@sentry/browser (incl. Tracing, Replay with Canvas)	74.66 KB (0%)
@sentry/browser (incl. Tracing, Replay, Feedback)	87.24 KB (0%)
@sentry/browser (incl. Tracing, Replay, Feedback, metrics)	89.08 KB (0%)
@sentry/browser (incl. metrics)	26.75 KB (0%)
@sentry/browser (incl. Feedback)	39.37 KB (0%)
@sentry/browser (incl. sendFeedback)	27.06 KB (0%)
@sentry/browser (incl. FeedbackAsync)	31.7 KB (0%)
@sentry/react	25.22 KB (0%)
@sentry/react (incl. Tracing)	37.22 KB (0%)
@sentry/vue	26.6 KB (0%)
@sentry/vue (incl. Tracing)	36.06 KB (0%)
@sentry/svelte	22.58 KB (0%)
CDN Bundle	23.64 KB (0%)
CDN Bundle (incl. Tracing)	35.88 KB (0%)
CDN Bundle (incl. Tracing, Replay)	70.26 KB (0%)
CDN Bundle (incl. Tracing, Replay, Feedback)	75.53 KB (0%)
CDN Bundle - uncompressed	69.37 KB (0%)
CDN Bundle (incl. Tracing) - uncompressed	106.31 KB (0%)
CDN Bundle (incl. Tracing, Replay) - uncompressed	217.95 KB (0%)
CDN Bundle (incl. Tracing, Replay, Feedback) - uncompressed	230.78 KB (0%)
@sentry/nextjs (client)	37.08 KB (0%)
@sentry/sveltekit (client)	34.81 KB (0%)
@sentry/node	111.92 KB (0%)
@sentry/node - without tracing	89.33 KB (+0.01% 🔺)
@sentry/aws-serverless	98.5 KB (0%)

andreiborza added the Package: solidstart Issues related to the Sentry SolidStart SDK label Jul 24, 2024

andreiborza requested review from Lms24, chargome and s1gr1d July 24, 2024 11:04

s1gr1d reviewed Jul 24, 2024

View reviewed changes

Lms24 approved these changes Jul 24, 2024

View reviewed changes

chargome approved these changes Jul 24, 2024

View reviewed changes

andreiborza added 2 commits July 25, 2024 09:47

feat(solidstart): Add server action instrumentation helper

33fa050

Remove unnecessary continueTrace calls

426ce94

andreiborza force-pushed the ab/solidstart-wrap-server-function branch from 281463d to 426ce94 Compare July 29, 2024 12:56

andreiborza requested review from chargome, Lms24, s1gr1d and mydea July 29, 2024 12:59

mydea reviewed Jul 29, 2024

View reviewed changes

Lms24 approved these changes Jul 29, 2024

View reviewed changes

Lms24 reviewed Jul 29, 2024

View reviewed changes

Rework origin and source for function spans

c0c9ffe

andreiborza requested review from mydea and Lms24 July 29, 2024 15:52

andreiborza added 2 commits July 30, 2024 08:59

Remove unused getTracePropagationData utility

977dd1c

Skip unit tests for node 16 as solidstart requires >=18

9aaa360

Lms24 approved these changes Jul 30, 2024

View reviewed changes

andreiborza merged commit 71af30f into develop Jul 30, 2024
127 checks passed

andreiborza deleted the ab/solidstart-wrap-server-function branch July 30, 2024 12:56

andreiborza mentioned this pull request Jul 30, 2024

Instrument SolidStart server actions and API routes #12549

Closed

sleidig mentioned this pull request Aug 21, 2024

[Snyk] Upgrade @sentry/angular from 8.20.0 to 8.21.0 Aam-Digital/ndb-core#2517

Closed

q1blue mentioned this pull request Aug 22, 2024

[Snyk] Upgrade @sentry/react from 7.16.0 to 8.21.0 https-quantumblockchainai-atlassian-net/ossinsight#814

Open

sleidig mentioned this pull request Aug 22, 2024

[Snyk] Upgrade @sentry/angular from 8.20.0 to 8.22.0 Aam-Digital/ndb-core#2519

Closed

angeloreale mentioned this pull request Aug 22, 2024

[Snyk] Upgrade @sentry/nextjs from 8.20.0 to 8.22.0 dreampipcom/hypnos#32

Merged

q1blue mentioned this pull request Aug 23, 2024

[Snyk] Upgrade @sentry/react from 7.16.0 to 8.22.0 https-quantumblockchainai-atlassian-net/ossinsight#816

Open

gokulk16 mentioned this pull request Aug 23, 2024

[Snyk] Upgrade @sentry/browser from 8.20.0 to 8.22.0 gokulk16/type-to-calculate-web#33

Closed

FengShangWuQi mentioned this pull request Aug 25, 2024

[Snyk] Upgrade @sentry/gatsby from 7.20.0 to 8.22.0 FengShangWuQi/fengshangwuqi.github.io#1916

Closed

angeloreale mentioned this pull request Aug 25, 2024

[Snyk] Upgrade @sentry/nextjs from 8.20.0 to 8.22.0 dreampipcom/hypnos#46

Closed

q1blue mentioned this pull request Aug 27, 2024

[Snyk] Upgrade @sentry/react from 7.16.0 to 8.23.0 https-quantumblockchainai-atlassian-net/ossinsight#820

Open

gokulk16 mentioned this pull request Aug 27, 2024

[Snyk] Upgrade @sentry/browser from 8.20.0 to 8.23.0 gokulk16/type-to-calculate-web#35

Closed

angeloreale mentioned this pull request Aug 27, 2024

[Snyk] Upgrade @sentry/nextjs from 8.20.0 to 8.23.0 dreampipcom/hygieia#1

Merged

tomwwinter mentioned this pull request Aug 27, 2024

[Snyk] Upgrade @sentry/angular from 8.20.0 to 8.23.0 Aam-Digital/ndb-core#2537

Closed

gokulk16 mentioned this pull request Aug 28, 2024

[Snyk] Upgrade @sentry/browser from 8.20.0 to 8.24.0 gokulk16/type-to-calculate-web#36

Closed

angeloreale mentioned this pull request Aug 28, 2024

[Snyk] Upgrade @sentry/nextjs from 8.20.0 to 8.24.0 dreampipcom/hygieia#3

Open

q1blue mentioned this pull request Aug 28, 2024

[Snyk] Upgrade @sentry/react from 7.16.0 to 8.24.0 https-quantumblockchainai-atlassian-net/ossinsight#833

Open

angeloreale mentioned this pull request Aug 29, 2024

[Snyk] Upgrade @sentry/nextjs from 8.20.0 to 8.24.0 dreampipcom/hygieia#4

Open

gokulk16 mentioned this pull request Aug 30, 2024

[Snyk] Upgrade @sentry/browser from 8.20.0 to 8.25.0 gokulk16/type-to-calculate-web#38

Merged

angeloreale mentioned this pull request Aug 30, 2024

[Snyk] Upgrade @sentry/nextjs from 8.20.0 to 8.25.0 dreampipcom/hygieia#5

Open

q1blue mentioned this pull request Aug 30, 2024

[Snyk] Upgrade @sentry/react from 7.16.0 to 8.25.0 https-quantumblockchainai-atlassian-net/ossinsight#835

Open

FengShangWuQi mentioned this pull request Sep 1, 2024

[Snyk] Upgrade @sentry/gatsby from 7.20.0 to 8.25.0 FengShangWuQi/fengshangwuqi.github.io#1918

Closed

PierreGode mentioned this pull request Sep 4, 2024

[Snyk] Upgrade @sentry/node from 8.9.2 to 8.25.0 PierreGode/shields#1

Open

q1blue mentioned this pull request Sep 7, 2024

[Snyk] Upgrade @sentry/react from 7.16.0 to 8.26.0 https-quantumblockchainai-atlassian-net/ossinsight#846

Open

FengShangWuQi mentioned this pull request Sep 8, 2024

[Snyk] Upgrade @sentry/gatsby from 7.20.0 to 8.26.0 FengShangWuQi/fengshangwuqi.github.io#1919

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(solidstart): Add server action instrumentation helper #13035

feat(solidstart): Add server action instrumentation helper #13035

andreiborza commented Jul 24, 2024 •

edited

Loading

s1gr1d Jul 24, 2024

andreiborza Jul 24, 2024

Lms24 left a comment

Lms24 Jul 24, 2024

andreiborza Jul 25, 2024

Lms24 Jul 29, 2024

chargome left a comment

andreiborza commented Jul 29, 2024

mydea Jul 29, 2024

andreiborza Jul 29, 2024

Lms24 Jul 29, 2024

mydea Jul 29, 2024

andreiborza Jul 29, 2024

andreiborza Jul 29, 2024

Lms24 Jul 29, 2024

andreiborza Jul 29, 2024

andreiborza Jul 30, 2024

Lms24 Jul 29, 2024

andreiborza Jul 29, 2024

mydea Jul 29, 2024

andreiborza Jul 29, 2024

github-actions bot commented Jul 29, 2024 •

edited

Loading

		const sentryTrace = (headers && headers.get('sentry-trace')) \|\| undefined;
		const baggage = (headers && headers.get('baggage')) \|\| null;

feat(solidstart): Add server action instrumentation helper #13035

feat(solidstart): Add server action instrumentation helper #13035

Conversation

andreiborza commented Jul 24, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Lms24 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chargome left a comment

Choose a reason for hiding this comment

andreiborza commented Jul 29, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Jul 29, 2024 • edited Loading

size-limit report 📦

andreiborza commented Jul 24, 2024 •

edited

Loading

github-actions bot commented Jul 29, 2024 •

edited

Loading