-
Notifications
You must be signed in to change notification settings - Fork 53
Add User-Agent Header to gRPC Metadata #417
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
|
|
||
| public static string GetUserAgent() | ||
| { | ||
| var version = typeof(DurableTaskUserAgentUtil).Assembly.GetName().Version; |
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.
This code gets the assembly version, which is different from the DLL version and isn't really what we want. Use this instead:
| var version = typeof(DurableTaskUserAgentUtil).Assembly.GetName().Version; | |
| string version = FileVersionInfo.GetVersionInfo(typeof(DurableTaskUserAgentUtil).Assembly.Location).FileVersion; |
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.
Also, make this into a readonly static value so that we read it only once and cache it rather than for every gRPC request.
static readonly string PackageVersion = FileVersionInfo.GetVersionInfo(typeof(DurableTaskUserAgentUtil).Assembly.Location).FileVersion;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.
fixed
…durabletask-dotnet into torosent/add_useragent
|
|
||
| public static class DurableTaskUserAgentUtil | ||
| { | ||
| static string SdkName => "durabletask-dotnet"; |
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.
| static string SdkName => "durabletask-dotnet"; | |
| const string SdkName => "durabletask-dotnet"; |
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.
fixed
| /// <summary> | ||
| /// Utility class for generating the user agent string for the Durable Task SDK. | ||
| /// </summary> | ||
| public static class DurableTaskUserAgentUtil |
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.
Why is this public and why is it in the shared code? Since this is only used by the AzureManaged package, shouldn't it go into that project?
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 moved it to the shared/azuremanaged
cgillum
left a comment
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.
Looks good. Please update the CHANGELOG.md file at the root of this repo and I can sign off.
This PR adds a user-agent header to the gRPC metadata in the DurableTaskSchedulerClientOptions.CreateChannel() method.
Why add a user-agent header?
DurableTaskUserAgentUtil.GetUserAgent()), making it easier to correlate issues or behaviors with specific client versions.Summary of changes
user-agentheader with the durabletask-dotnet SDK name and version to every outgoing request.This change improves observability and supportability for both SDK users and service operators.