Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Add TelemetryPolicy #210
Add TelemetryPolicy #210
Changes from 7 commits
2d1d028
a7a2589
d53f233
250c336
4075f13
a6e592d
0a70036
045f78f
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
The number of arguments here is getting pretty long. What speaks about
crate_name
andcrate_version
being inside ofClientOptions
?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 is temporary. For example, the retry policy needs to be removed in favor of retry options - higher-level abstracts that can pick the right policy (then keep those internal). But I can't use CARGO_* env vars from code in azure_core or they will always end up being "core" and azure_core's version. All our other SDKs pass this information in unless it can be retrieved automatically e.g. in .NET by checking for an assembly-level attribute.
It's probably worth using a builder at this point, which is what most of our other SDKs do. But that's something I want to tackle in a different PR. This PR is focused on the TelemetryPolicy specifically.
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 should also mention that most of the parameters you see here will actually be part of
ClientOptions
. In other languages, that's where we consistently provide telemetry, diagnostics (logging, mostly), retry options, and even the transport itself (optional override). For .NET, for example, since it can use reflection to get the name and version, it's only 3 parameters: options, and client-provided per-call and per-retry policies.