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

Add core version in UserAgent at all time #32242

Closed
wants to merge 1 commit into from
Closed

Conversation

lmazuel
Copy link
Member

@lmazuel lmazuel commented Sep 26, 2023

We want to always have the core version in the user-agent, for telemetry purposes.

This is a simple and stupid approach, meaning if calling SDK do not pass "sdk_monikers" you get the core version twice, but I think it's not important since:

  • We made sure all track2 were doing that, and not doing it is actually a bug in the SDK implementation
  • Codegen does it automatically

self._user_agent = "azsdk-python-{} Python/{} ({})".format(
sdk_moniker, platform.python_version(), platform.platform()
self._user_agent = "azsdk-python-{} {} Python/{} ({})".format(
sdk_moniker, core_version, platform.python_version(), platform.platform()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two scenarios it may impact:

  1. If the telemetry data parser uses strict schema, it may cause breakings.
  2. If user appends their ua string (by calling add_user_agent method or using env var), it may cause data loss.

Do we need to be concerned?

Copy link
Member

@kashifkhan kashifkhan Sep 26, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@xiangyan99 for # 2 would the data loss be around telemetry ( or others ) who are looking for the old UA string ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the ua string is too long, it will be truncated.

Because we insert rather than appending here, the most end data will be truncated which could be appended by user.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#1 is fair, maybe we put it at the end instead. @johanste any strong opinions here?
#2 I think is on the customer, there is no size for UserAgent in the HTTP spec, if people decided to truncate and read only a specific part, this is on them. Especially here we're adding roughly 12 bytes, which is not a lot.

@azure-sdk
Copy link
Collaborator

API change check

API changes are not detected in this pull request.

Copy link

github-actions bot commented Dec 1, 2023

Hi @lmazuel. Thank you for your interest in helping to improve the Azure SDK experience and for your contribution. We've noticed that there hasn't been recent engagement on this pull request. If this is still an active work stream, please let us know by pushing some changes or leaving a comment. Otherwise, we'll close this out in 7 days.

@github-actions github-actions bot added the no-recent-activity There has been no recent activity on this issue. label Dec 1, 2023
Copy link

github-actions bot commented Dec 8, 2023

Hi @lmazuel. Thank you for your contribution. Since there hasn't been recent engagement, we're going to close this out. Feel free to respond with a comment containing /reopen if you'd like to continue working on these changes. Please be sure to use the command to reopen or remove the no-recent-activity label; otherwise, this is likely to be closed again with the next cleanup pass.

@github-actions github-actions bot closed this Dec 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-recent-activity There has been no recent activity on this issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants