-
-
Notifications
You must be signed in to change notification settings - Fork 232
feat: Add PreferTransactionNameProvider option #5159
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
base: main
Are you sure you want to change the base?
Changes from 4 commits
527338b
f1eafab
4c42def
b484f6d
33c0d69
cb33bff
efa2d17
e9089eb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -178,13 +178,15 @@ public async Task InvokeAsync(HttpContext context) | |
| var status = SpanStatusConverter.FromHttpStatusCode(context.Response.StatusCode); | ||
|
|
||
| // If no Name was found for Transaction, then we don't have the route. | ||
| if (transaction.Name == string.Empty) | ||
| // Also, run this block if the caller has opted into always calling the TransactionNameProvider. | ||
| var customName = new Lazy<string?>(context.TryGetCustomTransactionName); | ||
| var forceCustomName = _options.AlwaysCallTransactionNameProvider && customName.Value is not null; | ||
| if (transaction.Name.Length == 0 || forceCustomName) | ||
|
jamescrosswell marked this conversation as resolved.
Outdated
jamescrosswell marked this conversation as resolved.
Outdated
|
||
| { | ||
| var method = context.Request.Method.ToUpperInvariant(); | ||
|
|
||
| // If we've set a TransactionNameProvider, use that here | ||
| var customTransactionName = context.TryGetCustomTransactionName(); | ||
| if (!string.IsNullOrEmpty(customTransactionName)) | ||
| if (customName.Value is { } customTransactionName) | ||
| { | ||
| transaction.Name = $"{method} {customTransactionName}"; | ||
| tracer.NameSource = TransactionNameSource.Custom; | ||
|
jamescrosswell marked this conversation as resolved.
Outdated
sentry[bot] marked this conversation as resolved.
Outdated
Comment on lines
+182
to
192
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bug: When Suggested FixIn the new code block (lines 182-192), add a guard to prevent overwriting the transaction name if it has already been manually modified. Before setting Prompt for AI Agent
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is intentional - the whole purpose of this PR is to address the gap highlighted in the original issue:
Anyone setting this option will have to check the existing transaction name (by reading the transaction from the scope) if they want to use that in the logic defined for returning a custom transaction name. |
||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.