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

Allow clients to tag requests with a source_tag #324

Merged
merged 11 commits into from Mar 22, 2024
Merged

Conversation

ssmith-pc
Copy link
Contributor

@ssmith-pc ssmith-pc commented Mar 14, 2024

Problem

Need to allow clients to include a source_tag to identify the source of their requests.

Solution

Allow clients to specify a source_tag field in the client constructor, that will be used to identify the traffic source, if applicable.

Example:

from pinecone import Pinecone

pc = Pinecone(api_key='foo', source_tag='bar')

pc.list_indexes()

This would cause the user-agent to get a value like:

User-Agent: 'python-client-3.1.0 (urllib3:2.0.7); source_tag=bar'

Type of Change

  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Testing

  • Tests are passing
  • Verify source_tag included in user-agent in control plane and data plane (REST and gRPC)

pinecone/config/config.py Outdated Show resolved Hide resolved
@ssmith-pc ssmith-pc requested a review from jhamon March 14, 2024 21:26
@jhamon
Copy link
Collaborator

jhamon commented Mar 14, 2024

Something else to keep in mind is we would like to track usage of SDKs coming from our own higher-level tools in addition to external integration partners. For example,

This doesn't change a ton about the general strategy, but it makes me think having partner in the name of these params isn't quite right.

@jhamon
Copy link
Collaborator

jhamon commented Mar 14, 2024

We should also reevaluate some of the cruft that's currently being passed in the user-agent strings. I've been here for a year and never had a need to know anything about what urllib3 version was in use.

@ssmith-pc
Copy link
Contributor Author

We should also reevaluate some of the cruft that's currently being passed in the user-agent strings. I've been here for a year and never had a need to know anything about what urllib3 version was in use.

That's good to know that there isn't much relying on what's there currently. Would be nice to identify the high value information we'd like to capture and include.

I can understand how it could be helpful to know what http client/version is being used to access the service, particularly if there is a known bug with a particular version that causes trouble for our service, as this would likely help CS triage and help.

@ssmith-pc
Copy link
Contributor Author

This doesn't change a ton about the general strategy, but it makes me think having partner in the name of these params isn't quite right.

Agree we should capture those clients, but I do wonder if we should try to keep the partner info separate. A lot of that depends on what we end up doing in the back-end, like how this info is extracted and matched up to what's in our partner CRM, etc.

In addition to setting the source_* fields, we could include a separate flag in our clients that identifies it as an internally owned integration.

@jhamon
Copy link
Collaborator

jhamon commented Mar 21, 2024

FYI there is a minimal unit test file called tests/unit/utils/test_user_agent.py that you may want to expand with a few more test cases to make sure the resulting string looks reasonable across a handful of different normalization cases.

@ssmith-pc
Copy link
Contributor Author

FYI there is a minimal unit test file called tests/unit/utils/test_user_agent.py that you may want to expand with a few more test cases to make sure the resulting string looks reasonable across a handful of different normalization cases.

I was working on that concurrently with your comment -- just pushed

@ssmith-pc ssmith-pc changed the title [DRAFT] Early POC for capturing partner source [DRAFT] Allow clients to tag requests with a source_tag Mar 21, 2024
@ssmith-pc ssmith-pc changed the title [DRAFT] Allow clients to tag requests with a source_tag Allow clients to tag requests with a source_tag Mar 21, 2024
@ssmith-pc ssmith-pc marked this pull request as ready for review March 21, 2024 19:19
Copy link
Collaborator

@jhamon jhamon left a comment

Choose a reason for hiding this comment

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

Approach and tests look good to me.

It can be tackled in a different PR if you want, but I think you'll need to go delving into GRPC base.py code and look at how additional_metadata gets passed. Unfortunately, I think those data calls are not sending the same user agent currently as gets sent with REST calls.

@ssmith-pc ssmith-pc merged commit 4fd2d20 into main Mar 22, 2024
125 checks passed
@ssmith-pc ssmith-pc deleted the ssmith/attribution_poc branch March 22, 2024 22:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants