Skip to content

openai: add OTEL_SDK_DISABLED tests - #107

Open
nehachinnasani23 wants to merge 1 commit into
elastic:mainfrom
nehachinnasani23:fix/openai-otel-sdk-disabled-tests
Open

openai: add OTEL_SDK_DISABLED tests#107
nehachinnasani23 wants to merge 1 commit into
elastic:mainfrom
nehachinnasani23:fix/openai-otel-sdk-disabled-tests

Conversation

@nehachinnasani23

@nehachinnasani23 nehachinnasani23 commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Add regression coverage for OpenAI chat completions and embeddings with OTEL_SDK_DISABLED=true.
  • Instrument with no-op trace and metric providers so the tests exercise non-recording spans and no-op metrics.
  • Reuse the existing VCR cassettes for chat and embeddings.

Closes #57

Testing

  • . .venv/bin/activate && cd instrumentation/elastic-opentelemetry-instrumentation-openai && pytest
  • . .venv/bin/activate && python -m compileall -q instrumentation/elastic-opentelemetry-instrumentation-openai/tests/test_otel_sdk_disabled.py

Note

  • python -m ruff check instrumentation/elastic-opentelemetry-instrumentation-openai/tests/test_otel_sdk_disabled.py was attempted, but ruff is not installed in the dev requirements environment.

@nehachinnasani23
nehachinnasani23 requested a review from a team as a code owner July 29, 2026 03:36
@cla-checker-service

cla-checker-service Bot commented Jul 29, 2026 •
edited
Loading

Copy link
Copy Markdown

💚 CLA has been signed

@nehachinnasani23

nehachinnasani23 commented Jul 29, 2026

Copy link
Copy Markdown
Contributor Author

Hi, I signed the Elastic Contributor Agreement after opening this PR. Could you please re-trigger or refresh the CLA check when you get a chance? Thank you.

@nehachinnasani23 nehachinnasani23 mentioned this pull request Jul 29, 2026
Open
xrmx
xrmx requested changes Aug 13, 2026
View reviewed changes

@xrmx xrmx left a comment •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for the PR. We plan to deprecate this instrumenation in favor of the upstream openai one from https://github.com/open-telemetry/opentelemetry-python-genai/ that exports a newer semantic convention. Anyway if you update this PR I'll merge it gladly. Are you using this instrumentation?

Comment thread instrumentation/elastic-opentelemetry-instrumentation-openai/tests/test_otel_sdk_disabled.py
Comment on lines +29 to +30
def otel_sdk_disabled_instrumentor(monkeypatch, instrument):
instrument.uninstrument()

@xrmx xrmx Aug 13, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

With the other PR merged you don't need to do this anymore. Also I would add this in conftests.py with the others and name it otel_sdk_disabled_instrument.

Comment thread instrumentation/elastic-opentelemetry-instrumentation-openai/tests/test_otel_sdk_disabled.py
Comment on lines +35 to +36
tracer_provider=trace.NoOpTracerProvider(),
meter_provider=metrics.NoOpMeterProvider(),

@xrmx xrmx Aug 13, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Are these required?

Comment thread instrumentation/elastic-opentelemetry-instrumentation-openai/tests/test_otel_sdk_disabled.py
messages=[{"role": "user", "content": TEST_CHAT_INPUT}],
)

assert response.choices[0].message.content == "Atlantic Ocean."

@xrmx xrmx Aug 13, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Will something like the following work (after you inject tracer_exporter)?

    assert not trace_exporter.get_finished_spans()

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@xrmx xrmx xrmx requested changes

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

openai: add tests with OTEL_SDK_DISABLED=true

2 participants

@nehachinnasani23 @xrmx