-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
ref(serverless): Add transaction source #5394
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
Conversation
size-limit report 📦
|
@@ -87,7 +87,7 @@ function _wrapHttpFunction(fn: HttpFunction, wrapOptions: Partial<HttpFunctionWr | |||
name: `${reqMethod} ${reqUrl}`, | |||
op: 'gcp.function.http', | |||
...traceparentData, | |||
metadata: { baggage }, | |||
metadata: { baggage, source: 'url' }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps dumb question, but is parameterization even a thing in this context?
I'm not super familiar with GCP, but it seems from this and this and this like data can only be passed either in the query string or in the request body.
If that's true, then I think we can switch this to 'route'
, because it then isn't high cardinality. (That might not be strictly true, because the region
part of the URL might vary, but we can just substitute a parameter there if we want.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mostly just defaulted to url here to stay on the safe side - but I agree with your reasoning, will change to route
.
cda2aae
to
3590cc2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
ref: #5345
Decided to go with
component
for the serverless functions as they are function names as transaction name.