open-telemetry / opentelemetry-python-contrib

OpenTelemetry instrumentation for Python modules
https://opentelemetry.io
Apache License 2.0
739 stars 612 forks source link

opentelemetry-instrumentation-openai-v2: scrub cookie from tests #2993

Closed codefromthecrypt closed 1 week ago

codefromthecrypt commented 1 week ago

Description

This scrubs test data and rewords token replacements corresponding to canonical ENV variables.

Before, we had inconsistent test data where most sensitive properties were scrubbed, but not all. The main thing this does is scrub out the request cookie, so that folks don't accidentally leak it in new PRs. After that, it helps clarify the source of sensitive data by making the replacement tokens match naming conventions of OpenAI ENV variables.

Also, before we scrubbed "api-key", but that is only set when using AzureClient, which this package doesn't use, yet. When it does, we should probably scrub more (as the URLs include sensitive project-specific details). If we end up supporting AzureClient, I can help in the scrub config for that.

Noticed the cookie problem in #2984 See https://github.com/openai/openai-python/blob/646a579cdb305a9d3fba6c5f9a96011c5e2c2882/src/openai/_client.py#L98-L100

Type of change

Please delete options that are not relevant.

How Has This Been Tested?

I deleted all test yaml and ran the test two ways to make sure it recreates correctly, with no cookies in the yaml

Does This PR Require a Core Repo Change?

Checklist:

See contributing.md for styleguide, changelog guidelines, and more.

alizenhom commented 1 week ago

lgtm @codefromthecrypt

codefromthecrypt commented 1 week ago

Thanks @alizenhom can you put the skip changelog label on for me? I don't have karma for that