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 sourceTag to PineconeConfiguration and additionalHeaders to data plane calls #197

Merged
merged 5 commits into from
Mar 27, 2024

Conversation

austin-denoble
Copy link
Contributor

@austin-denoble austin-denoble commented Jan 25, 2024

Problem

We want to provide the ability for consumers to include a source_tag to identify the source of requests while using the client. Additionally, there are debugging and development scenarios where being able to pass additional headers with network requests would be convenient.

Solution

  • Add sourceTag to PineconeConfiguration. Update buildUserAgent() to take in PineconeConfiguration, which should be available at all the points we call to build the agent. If sourceTag is provided it will be normalized, and inserted into the User-Agent header for all network requests with this format: source_tag=<normalizeSourceTag>.
    • Normalization rules:
    • Lowercase
    • Limit to charset [a-z0-9_ ]
    • Trim left/right space
    • Condense all spaces to one space and replace with _
  • Add additionalHeaders to the Index constructor. Custom headers can now be provided when targeting a specific index. Index now passes these headers to VectorOperationsProvider when setting up the API.
  • Update VectorOperationsProvider to apply additionalHeaders to the API Configuration when provided.

Type of Change

New feature, but specific to developer / support usage.

  • New feature (non-breaking change which adds functionality)

Test Plan

New unit test files:

  • indexOperationsBuilder.test.ts
  • user-agent.test.ts

New unit test for validating additionalHeaders is passed as expected, a bit of refactoring in index.test.ts which appears messier than it is.

Running locally with PINECONE_DEBUG=true to make sure the custom headers / sourceTag are passed on requests when provided.

Passing integrationId

import { Pinecone } from '@pinecone-database/pinecone'

const pc = new Pinecone({ apiKey: 'your-api-key', sourceTag: 'test source tag' });
const index = pc.Index('my-index');

// Test control + data plane operations
await pc.listIndexes();
await index.upsert(...);

Passing additionalHeaders to Index

import { Pinecone } from '@pinecone-database/pinecone'

const pc = new Pinecone({ apiKey: 'your-api-key', sourceTag: 'test source tag' });
const index = pc.Index('my-index', undefined, { 'x-custom-header': 'header-value' });

// Make requests
await index.upsert(...);

@ssmith-pc
Copy link

This looks directionally in line with what's needed for PRD Integration Attributes - SDK Attribution.

Related PR from Python client: pinecone-io/pinecone-python-client#324

@austin-denoble austin-denoble force-pushed the adenoble/additional-headers-data-plane branch from b723896 to 79cc4ea Compare March 26, 2024 18:10
@austin-denoble austin-denoble changed the title Add integrationId to PineconeConfiguration and additionalHeaders to data plane calls Add sourceTag to PineconeConfiguration and additionalHeaders to data plane calls Mar 26, 2024
Copy link

@haruska haruska left a comment

Choose a reason for hiding this comment

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

Looks good. Is there a corresponding docs (or README) update explaining the new config?

src/utils/user-agent.ts Show resolved Hide resolved
Copy link

@ssmith-pc ssmith-pc left a comment

Choose a reason for hiding this comment

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

LGTM overall

src/utils/user-agent.ts Show resolved Hide resolved
@austin-denoble austin-denoble merged commit b386099 into main Mar 27, 2024
25 checks passed
@austin-denoble austin-denoble deleted the adenoble/additional-headers-data-plane branch March 27, 2024 16:49
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.

3 participants