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

Can not add sentry-trace header to every fetch #3169

Closed
4 tasks
batamar opened this issue Jan 12, 2021 · 19 comments
Closed
4 tasks

Can not add sentry-trace header to every fetch #3169

batamar opened this issue Jan 12, 2021 · 19 comments

Comments

@batamar
Copy link

batamar commented Jan 12, 2021

Package + Version

  • [5.29.2 ] @sentry/browser
  • [ 5.29.2] @sentry/node
  • raven-js
  • raven-node (raven for node)
  • other:

Description

For this request this added
Screen Shot 2021-01-12 at 22 59 13

But for next several request it is not adding

Screen Shot 2021-01-12 at 22 59 47

@t-h-man
Copy link
Member

t-h-man commented Jan 18, 2021

Can you share the SDK init?

@kamilogorek
Copy link
Contributor

sentry-trace header is only attached, if there is an active transaction happening. If Sentry.getScope().getSpan() returns null (which is equivalent to transaction not being active), this operation is skipped.
Your first request has the header because initial page load and page navigations are creating transactions, which finish once the operation completes.

Responsible code:

const scope = getCurrentHub().getScope();
if (scope && tracingEnabled) {
parentSpan = scope.getSpan();
if (parentSpan) {
span = parentSpan.startChild({
description: `${requestOptions.method || 'GET'} ${requestUrl}`,
op: 'request',
});
const sentryTraceHeader = span.toTraceparent();
logger.log(`[Tracing] Adding sentry-trace header to outgoing request: ${sentryTraceHeader}`);
requestOptions.headers = { ...requestOptions.headers, 'sentry-trace': sentryTraceHeader };
}
}

const activeTransaction = getActiveTransaction();
if (activeTransaction) {
const span = activeTransaction.startChild({
data: {
...handlerData.fetchData,
type: 'fetch',
},
description: `${handlerData.fetchData.method} ${handlerData.fetchData.url}`,
op: 'http',
});
handlerData.fetchData.__span = span.spanId;
spans[span.spanId] = span;
const request = (handlerData.args[0] = handlerData.args[0] as string | Request);
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const options = (handlerData.args[1] = (handlerData.args[1] as { [key: string]: any }) || {});
let headers = options.headers;
if (isInstanceOf(request, Request)) {
headers = (request as Request).headers;
}
if (headers) {
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
if (typeof headers.append === 'function') {
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
headers.append('sentry-trace', span.toTraceparent());
} else if (Array.isArray(headers)) {
headers = [...headers, ['sentry-trace', span.toTraceparent()]];
} else {
headers = { ...headers, 'sentry-trace': span.toTraceparent() };
}
} else {
headers = { 'sentry-trace': span.toTraceparent() };
}
options.headers = headers;
}

@kamilogorek
Copy link
Contributor

Closing the issue as a part of large repository cleanup, due to it being inactive and/or outdated. There's a large chance that this issue has been solved since.
Please do not hesitate to ping me if it is still relevant, and I will happily reopen and work on it.
Cheers!

@laygir
Copy link

laygir commented Oct 15, 2021

Hey there,

I've been trying the tracing/performance features of Sentry recently, and I noticed this behaviour on my end.
Not every http requests have the sentry-trace header.

I have been in touch with Sentry support but it is taking too long, so I thought I would approach here as well.

Sentry support wrote me the following statements in two different emails:

  • "if a transaction doesn't exist, no sentry-trace header will be attached to a request."
  • "For the front-end side, the automatic instrumentation should create transaction to every XHR request (as long as the sampling is 100%)."

Above statements feel a bit contradictry to me, if every XHR request creates a transaction, how come the header is not attached at the time of a request.

I've already shared my debug logs with the support and last email received from the support is:
"I'm partnering with a SDK engineer for this and I will get back to you as soon as I have an update."

And if somehow support made a wrong statement and header is only attached if an active transaction exists and not every xhr request creates a transaction, then I want to ask how limited is the automatic instrumentation?

Do I need to manually create a transaction for a possible interaction to have it traced through frontend and backend? Isn't there an option to create a transaction automatically when an XHR request about to happen without me manually trying to figure out if a transaction exists at the time of the request and if not create it every time?

For example on initial page load, we have about 8 api calls to our backend and sometimes 4 some times 3 out of 8 has the necessary header. Specially the first 3 calls made never has the header.

Then for example there is a button on the ui that calls an endpoint and backend rejects it. There is no trace header on the request so we are not able to follow the event to the backend. Event is visible individually for frontend and backend. But we do not have a breakdown the complete picture.

Just in case; my sampling is set to 1.0 and I'm really hoping I'm doing something wrong and there is a way to add the header to all xhr request for the given origins without doing manual work there..

I'm using the following packages:

"@sentry/tracing": "^6.13.3"
"@sentry/vue": "^6.13.3"

@fliespl
Copy link

fliespl commented Dec 5, 2021

@kamilogorek can you explain how to attach next XHR / Fetch events after pageload transaction finishes?

I am getting pageload transaction logged, but I also want to metric later on xhr/fetch requests (which of course don't have sentry-trace header cause transaction is gone).

@ydennisy
Copy link

@kamilogorek this is still an active issue - please could you re-open and provide a workaround for Vue?

@kamilogorek
Copy link
Contributor

@ydennisy please open a new issue so that team can triage it, thanks!

@adzza24
Copy link

adzza24 commented Jun 21, 2022

Was there a new ticket for this one? Still seeing this issue, even when I manually create and open a transaction before the XHR request and don't close it until the response is received. Even with tracesSampleRate set to 1.0 I still only ever get sentry-trace and baggage headers added on the first 2 requests which run whilst the page is loading.

@yarinsa
Copy link

yarinsa commented Aug 17, 2022

Was this ever resolved? I am not seeing sentry-trace on any of my requests

@mmahalwy
Copy link

Found the solution. Was working in development but not in prod. Then I discovered it's because you have to see the tracingOrigins yourselves and localhost is the default. See:
https://github.com/getsentry/sentry-javascript/blob/master/packages/tracing/src/browser/request.ts#L17-L23

@laygir
Copy link

laygir commented Aug 24, 2022

@mmahalwy I hope that was the issue with your situation.

But I think it is not the solution..
I have inserted right values in the tracingOrigins and behaviour was still the same for me.

The problem appears to be that Sentry only attaches the header if there is an ongoing transaction.
Since transactions don't keep open, not all your requests will have the header.

I guess you could work around this and create a transaction every time you are about to make a fetch request, but this is not nice considering what they called "automated instrumentation". If I need to handle all that stuff, how is that an automated instrumentation.. Marketing bs..
And what they offer as APM is not an APM.. Compare it with what you get from New Relic and see your self which one is an actual APM. (from backend services point of view)

I have the following issue created as this one was closed. And it is marked as bug by the Sentry team..
#4072

But still, even after 3 written assurances by email from 1 sales person and 2 times by a support person that this issue was going to be fixed in "a few weeks", we are approaching the anniversary of it and still unresolved.

Company I'm working in is a paid customer for 5 years and because this has been an issue for us, we are not able upgrade our account because we can't use these transactions without header as they don't get associated with other transactions, there is no point in that..

So I'm sure there is subscription/growth attached to this problem for the product and Sentry is choosing not to fix it.

My 5 cents on this: the last few years they got some investments coming in and since then the dynamics in the company are changing and this reflects on the product obviously..

@mmahalwy
Copy link

@laygir i actually have the same problem. Only some requests are sending but not all due to current transaction.

@mattkindy
Copy link

I see this issue as well

@AbhiPrasad
Copy link
Member

Hey folks - just following up on this thread.

@laygir we apologize for the delay in getting back - but we are a small team, we can only work on so much at a time. The past few months we've been focusing on reducing bundle size, adding support for our dynamic sampling/server-side sampling feature, and releasing SDKs for Svelte and Remix. The bundle size work was most important to us, so we've been purposely putting our energies toward that. That has been wrapped up, so we can now move onto other things!

Now onto the current issue. To solve this, we would have to figure out what transaction to place these requests in. It doesn't make sense to extend our pageload or navigation transactions, because often times these requests are not part of the "page render" operation. They happen due to another actions that happens outside of it. For example, a button click or a background poll.

My question to you all then - what kind of transaction should we auto-instrument to have a trace? What should this be called, what kind of information would you like to see in it? If I had to put my horse in a race, it would be cool to get user interactions, like clicks, auto-instrumented - this is similar to the premise that you put forward @laygir. I'll try writing up a new issue with details about this, and maybe we can scope this out here.

@yarinsa
Copy link

yarinsa commented Sep 19, 2022

@AbhiPrasad I would want to create a transaction starting from the "load route" event of the router until it's done loading.
I would want to know:

  1. Which network requests initiated by this route component
  2. How long did it take to load the script
  3. How long did it take until there was an actual UI to see

My use case:
I have a table of applications kept in the DB. I would like to trace the time it takes to load the page and show the apps.
(We added manual monitoring to the request size), but I would want to identify that there is a problem.

  • Integrating with React.Lazy would be amazing as well.

@AbhiPrasad
Copy link
Member

I would want to create a transaction starting from the "load route" event of the router until it's done loading

What framework is this with? We should be capturing any http requests as part of load (as long as the tracingOrigins is being set up correctly) in our navigation routes.

@yarinsa
Copy link

yarinsa commented Sep 19, 2022

As you can see here it's like 4-5 transactions combined. Starting from page load, all the way to the error. As you can see the user changed routes multiple times. We end up with a very long transaction. Which started on "first load"

Screen.Recording.2022-09-19.at.17.53.54.mov

@mattkindy
Copy link

Hey folks - just following up on this thread.

@laygir we apologize for the delay in getting back - but we are a small team, we can only work on so much at a time. The past few months we've been focusing on reducing bundle size, adding support for our dynamic sampling/server-side sampling feature, and releasing SDKs for Svelte and Remix. The bundle size work was most important to us, so we've been purposely putting our energies toward that. That has been wrapped up, so we can now move onto other things!

Now onto the current issue. To solve this, we would have to figure out what transaction to place these requests in. It doesn't make sense to extend our pageload or navigation transactions, because often times these requests are not part of the "page render" operation. They happen due to another actions that happens outside of it. For example, a button click or a background poll.

My question to you all then - what kind of transaction should we auto-instrument to have a trace? What should this be called, what kind of information would you like to see in it? If I had to put my horse in a race, it would be cool to get user interactions, like clicks, auto-instrumented - this is similar to the premise that you put forward @laygir. I'll try writing up a new issue with details about this, and maybe we can scope this out here.

I would love to see user interactions auto-instrumented, although I recognise that's probably not simple. That's probably our main use case, though. I think there are also cases where events fire outside of the scope of the original pageload request and are not caught. An example of this might be when using something like react-query and the local cache expires, resulting in an API call.

Right now I am working around this by adding axios request/response interceptors which creates a transaction if missing:

axiosInstance.interceptors.request.use(async request => {
  try {
    const existingTransaction = Sentry.getCurrentHub().getScope()?.getTransaction();

    // Create a transaction if one does not exist in order to work around
    // https://github.com/getsentry/sentry-javascript/issues/3169
    // https://github.com/getsentry/sentry-javascript/issues/4072
    const transaction =
      existingTransaction ??
      Sentry.startTransaction({ name: `API Request: ${request.method} ${request.url}` });

    Sentry.getCurrentHub().configureScope(scope => scope.setSpan(transaction));

    (request as any).tracing = {
      transaction,
    };

    return request;
  } catch (err) {
    console.error(err);
    Sentry.captureException(err);
    return request;
  }
});

axiosInstance.interceptors.response.use(
  response => {
    (response.config as any).tracing?.transaction?.finish();
    return response;
  },
  error => {
    (error.config as any).tracing?.transaction?.finish();
    return error;
  }
);

I don't love this approach since a group of API requests will

  1. End up in the transaction of the API request first fired, meaning the ability to search/group performance is more inconsistent
  2. Not be grouped under a single transaction that represents the original "trigger" for the API requests, making it harder to answer questions as they relate to performance

@yarinsa
Copy link

yarinsa commented Sep 21, 2022

2. they

@mattkindy That looks good! We are having the same issue, can you share what the output on Sentry looks like?

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

No branches or pull requests