fix: migrate from events api to log records for otel 1.37.0+ compatibility #3453
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughReplace OpenTelemetry Event/EventLogger APIs with LogRecord/Logger/get_logger across instrumentations, rename event_logger_provider → logger_provider, update tests to use log_record.event_name, bump OpenTelemetry dependency ranges, add Bedrock header-based token counts, and relax AlephAlpha client upper bound. Changes
Sequence Diagram(s)sequenceDiagram
participant App as Instrumented Code
participant Wrap as Wrapper (_wrap/_awrap)
participant Emitter as EventEmitter
participant Logger as Logger
Note over App,Logger: OLD (Event-based)
App->>Wrap: call instrumented function
Wrap->>Emitter: emit_event(event, event_logger)
Emitter->>Logger: emit(Event(name, body, attributes))
Note over App,Logger: NEW (LogRecord-based)
App->>Wrap: call instrumented function
Wrap->>Emitter: emit_event(event, logger)
Emitter->>Logger: create LogRecord(body, attributes, event_name)
Emitter->>Logger: emit(log_record)
sequenceDiagram
participant Tests as Test Code
participant LogRec as LogRecord
Note over Tests,LogRec: OLD (attributes)
Tests->>LogRec: read attributes.get(EventAttributes.EVENT_NAME)
LogRec-->>Tests: event name
Note over Tests,LogRec: NEW (direct field)
Tests->>LogRec: read event_name
LogRec-->>Tests: event name
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Changes requested ❌
Reviewed everything up to 35c0ba1 in 7 minutes and 26 seconds. Click for details.
- Reviewed
4581lines of code in144files - Skipped
32files when reviewing. - Skipped posting
2draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. packages/opentelemetry-semantic-conventions-ai/pyproject.toml:26
- Draft comment:
Dependency versions updated to ^1.38.0 for opentelemetry-sdk and ^0.59b0 for opentelemetry-semantic-conventions appear correct for LogRecords support. No issues found in this file. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is purely informative and does not provide any actionable feedback or suggestions. It simply states that the dependency versions appear correct and that no issues were found, which violates the rule against making purely informative comments.
2. packages/traceloop-sdk/pyproject.toml:18
- Draft comment:
The repository URL appears to have a typo ('openllmetry' instead of 'opentelemetry'). Please verify if this is intentional. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
Workflow ID: wflow_OfLSJ5FWqlV4MLjy
You can customize 
| event_logger_provider = kwargs.get("event_logger_provider") | ||
| event_logger = get_event_logger( | ||
| __name__, __version__, event_logger_provider=event_logger_provider | ||
| logger_provider= kwargs.get("logger_provider") |
There was a problem hiding this comment.
Typo: There is a missing space around the assignment operator on line 606. Consider changing logger_provider= kwargs.get("logger_provider") to logger_provider = kwargs.get("logger_provider") for consistency.
| logger_provider= kwargs.get("logger_provider") | |
| logger_provider = kwargs.get("logger_provider") |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (7)
packages/opentelemetry-instrumentation-bedrock/tests/traces/test_meta.py (1)
5-6: Remove unused EventAttributes import.The search confirms that
EventAttributesis imported on line 5-6 but never used in the file. After the migration to directevent_namefield access, this import is now unused and should be removed.packages/opentelemetry-instrumentation-openai/tests/traces/test_vision.py (1)
8-9: Remove the unusedEventAttributesimport on line 8.The
EventAttributesimport is no longer referenced after migrating to direct access ofevent_namefrom the log record. Flake8 will flag this as an unused import.packages/opentelemetry-instrumentation-transformers/pyproject.toml (1)
38-38: Update dev dependency to match main API version requirement.The dev SDK version
^1.34.1is incompatible with the package's instrumentation code. Theevent_nameparameter used inLogRecord()calls (lines 113, 145 of event_emitter.py) was added in v1.35.0, but v1.34.1 predates this change. This causes a version mismatch between dev testing (v1.34.1, missingevent_namesupport) and production (v1.38.0 API requirement, requiresevent_namesupport). Update toopentelemetry-sdk = "^1.38.0"to match the main API requirement and ensure test behavior aligns with production.All other instrumentation packages in this repository already use
^1.38.0for the SDK test dependency.packages/opentelemetry-instrumentation-watsonx/opentelemetry/instrumentation/watsonx/event_emitter.py (1)
32-49: Guard againstevent_loggerbeingNonebefore delegating to_emit_*
emit_event’s signature explicitly allowsevent_logger: Union[Logger, None], but_emit_message_eventand_emit_choice_eventboth type it asLoggerand callevent_logger.emit(...)unconditionally. Ifemit_eventis ever invoked withevent_logger=None, this will raiseAttributeError: 'NoneType' object has no attribute 'emit'.Either make
event_loggernon‑optional everywhere, or (simpler/safer) short‑circuit when it isNoneso internal helpers always see a realLogger.You can add a guard like this to keep the public API optional while avoiding runtime errors:
def emit_event( event: Union[MessageEvent, ChoiceEvent], event_logger: Union[Logger, None] ) -> None: @@ - if not should_emit_events(): - return + if not should_emit_events(): + return + + # If no logger is configured, there is nothing to emit. + if event_logger is None: + return if isinstance(event, MessageEvent): _emit_message_event(event, event_logger) elif isinstance(event, ChoiceEvent): _emit_choice_event(event, event_logger)packages/opentelemetry-instrumentation-watsonx/opentelemetry/instrumentation/watsonx/__init__.py (2)
490-499: Fix_handle_inputsignature to match call site and avoid disabling input instrumentation
_handle_inputis defined with an unusedresponse_counterparameter:def _handle_input(span, event_logger, name, instance, response_counter, args, kwargs):but called as:
_handle_input(span, event_logger, name, instance, args, kwargs)Because of
@dont_throw, this mismatch will raise inside the wrapper and be swallowed, effectively skipping input attribute/event handling.You can remove the unused parameter to align the signature with the call:
-@dont_throw -def _handle_input(span, event_logger, name, instance, response_counter, args, kwargs): +@dont_throw +def _handle_input(span, event_logger, name, instance, args, kwargs): @@ - if should_emit_events() and event_logger: - _emit_input_events(args, kwargs, event_logger) + if should_emit_events() and event_logger: + _emit_input_events(args, kwargs, event_logger)No changes are needed at the call site.
Also applies to: 581-581
351-360: Ensure response events use the logger correctly and fix_emit_response_eventssignatureCurrently:
_emit_response_eventsis declared asdef _emit_response_events(response: dict):and only takes the response._handle_responsecalls_emit_response_events(responses, event_logger)._handle_stream_responsecalls_emit_response_events({...})without the logger.- Inside
_emit_response_events,emit_event(ChoiceEvent(...))is called withoutevent_logger.This creates a positional-argument mismatch at runtime and also prevents completion events from being associated with the configured logger.
A minimal fix is to thread
event_loggerthrough:-def _emit_response_events(response: dict): +def _emit_response_events(response: dict, event_logger): for i, message in enumerate(response.get("results", [])): - emit_event( + emit_event( ChoiceEvent( index=i, message={"content": message.get("generated_text"), "role": "assistant"}, finish_reason=message.get("stop_reason", "unknown"), ) - ) + , event_logger) @@ def _handle_response( @@ if should_emit_events() and event_logger: - _emit_response_events(responses, event_logger) + _emit_response_events(responses, event_logger) @@ def _handle_stream_response( span, event_logger, stream_response, stream_generated_text, stream_stop_reason ): @@ - if should_emit_events() and event_logger: - _emit_response_events( - { - "results": [ - { - "stop_reason": stream_stop_reason, - "generated_text": stream_generated_text, - } - ] - }, - ) + if should_emit_events() and event_logger: + _emit_response_events( + { + "results": [ + { + "stop_reason": stream_stop_reason, + "generated_text": stream_generated_text, + } + ] + }, + event_logger, + )This restores proper logging for both non‑streaming and streaming responses and removes the call/signature mismatch.
Also applies to: 362-371, 503-518, 530-547
packages/opentelemetry-instrumentation-anthropic/opentelemetry/instrumentation/anthropic/event_emitter.py (1)
167-217: Guard log emission when the logger is absent
AnthropicInstrumentor._handle_response(Line 505) and_ahandle_response(Line 490) callemit_response_events(event_logger, …)even whenevent_loggerisNone(e.g., legacy attributes enabled). With the new code path,emit_eventnow unconditionally forwards thatNoneinto_emit_message_event/_emit_choice_event, and the subsequentevent_logger.emit(...)raises anAttributeError. Please short‑circuit when the logger is missing (and reflect the Optional type) so legacy configurations do not crash.Apply this diff:
-def emit_event( - event: Union[MessageEvent, ChoiceEvent], event_logger: Logger -) -> None: +def emit_event( + event: Union[MessageEvent, ChoiceEvent], event_logger: Optional[Logger] +) -> None: @@ - if not should_emit_events(): + if not should_emit_events() or event_logger is None: return
🧹 Nitpick comments (30)
packages/opentelemetry-instrumentation-transformers/tests/test_pipeline.py (1)
4-6: Remove unused import.The
EventAttributesimport is no longer used after migrating to the LogRecords API, where event names are accessed via theevent_namefield rather than attributes.Apply this diff to remove the unused import:
-from opentelemetry.semconv._incubating.attributes import ( - event_attributes as EventAttributes, -)packages/opentelemetry-instrumentation-together/tests/test_chat.py (1)
3-5: Remove unused EventAttributes import.After migrating to the LogRecords API where
event_nameis accessed directly fromlog_record.event_name(line 140), theEventAttributesimport is no longer used in this file.Apply this diff to remove the unused import:
-from opentelemetry.semconv._incubating.attributes import ( - event_attributes as EventAttributes, -) from opentelemetry.semconv._incubating.attributes import ( gen_ai_attributes as GenAIAttributes, )packages/opentelemetry-instrumentation-ollama/tests/conftest.py (1)
93-95: Remove unusedreaderparameter.The
readerparameter is declared in both fixtures but never used in the function body.Apply this diff:
@pytest.fixture(scope="function") def instrument_with_content( - reader, tracer_provider, logger_provider, meter_provider + tracer_provider, logger_provider, meter_provider ): os.environ.update({TRACELOOP_TRACE_CONTENT: "True"}) instrumentor = OllamaInstrumentor(use_legacy_attributes=False) instrumentor.instrument( tracer_provider=tracer_provider, logger_provider=logger_provider, meter_provider=meter_provider, ) yield instrumentor os.environ.pop(TRACELOOP_TRACE_CONTENT, None) instrumentor.uninstrument() @pytest.fixture(scope="function") def instrument_with_no_content( - reader, tracer_provider, logger_provider, meter_provider + tracer_provider, logger_provider, meter_provider ):Also applies to: 112-114
packages/opentelemetry-instrumentation-ollama/opentelemetry/instrumentation/ollama/event_emitter.py (1)
187-193: Fix formatting: remove trailing blank line.There's an unnecessary blank line at line 191 before the closing parenthesis.
Apply this diff:
log_record = LogRecord( body=body, attributes=EVENT_ATTRIBUTES, event_name="gen_ai.choice" - ) event_logger.emit(log_record)packages/opentelemetry-instrumentation-ollama/opentelemetry/instrumentation/ollama/__init__.py (1)
10-10: Remove unusedLoggerimport.The
Loggertype is imported but never used in the code.Apply this diff:
-from opentelemetry._logs import Logger, get_logger +from opentelemetry._logs import get_loggerpackages/opentelemetry-instrumentation-sagemaker/opentelemetry/instrumentation/sagemaker/event_emitter.py (2)
84-86: Consider renamingevent_loggerparameter for consistency.The parameter is now typed as
Loggerinstead ofEventLogger, but the parameter name still includes "event_logger". While this doesn't affect functionality, renaming it tologgerwould better reflect the new API.Apply this diff if you'd like to improve naming consistency:
-def emit_event( - event: Union[MessageEvent, ChoiceEvent], event_logger: Union[Logger, None] -) -> None: +def emit_event( + event: Union[MessageEvent, ChoiceEvent], logger: Union[Logger, None] +) -> None:Then update the parameter name throughout the function bodies accordingly.
Also applies to: 104-104, 136-136
152-158: Remove unnecessary blank line in constructor call.The LogRecord construction is functionally correct, but there's an unnecessary blank line (line 156) before the closing parenthesis.
Apply this diff to clean up the formatting:
log_record = LogRecord( body=body, attributes=EVENT_ATTRIBUTES, event_name="gen_ai.choice" - ) event_logger.emit(log_record)packages/opentelemetry-instrumentation-openai/tests/traces/test_azure.py (1)
7-7: Remove unusedEventAttributesimport from line 7.Verification confirms
EventAttributesis imported but never used in the file. Remove this unused import to comply with Flake8 rules (F401).-from opentelemetry.semconv._incubating.attributes import ( - event_attributes as EventAttributes, -)packages/opentelemetry-instrumentation-writer/pyproject.toml (1)
24-24: Consider stabilizing beta version constraints.The floor constraints
>=0.59b0are unusually permissive for beta versions. If the intent is to support the stable 0.59.0 release and beyond, consider pinning to^0.59.0or>=0.59.0for more predictable dependency resolution, consistent with line 23's caret constraint on the API version.-opentelemetry-instrumentation = ">=0.59b0" -opentelemetry-semantic-conventions = ">=0.59b0" +opentelemetry-instrumentation = "^0.59.0" +opentelemetry-semantic-conventions = "^0.59.0"Also applies to: 25-25
packages/opentelemetry-instrumentation-writer/tests/test_chat.py (1)
13-21: Assert helper is correct; consider centralizing to avoid duplicationReading
log.log_record.event_namehere matches the new LogRecord emission logic, and the body/attribute checks are consistent with the completions tests. Since this helper is duplicated across writer tests, consider moving it into a shared test utility to keep future changes to the log format in one place.packages/opentelemetry-instrumentation-writer/tests/conftest.py (1)
45-50: Logger provider wiring looks good; drop unusedreaderargs to satisfy lintingThe new
logger_providerfixture correctly initializes aLoggerProviderand attachesSimpleLogRecordProcessor(log_exporter), and passinglogger_provider=logger_providerintoWriterInstrumentor.instrumentis consistent with the Logger-based API. Thereaderparameters oninstrument_with_contentandinstrument_with_no_contentare unused, though, and Ruff flags them (ARG001); sincemeter_provideralready depends onreader, you can safely removereaderfrom these fixture signatures to keep the fixtures clean and lint‑friendly.Also applies to: 91-103, 110-121
packages/opentelemetry-instrumentation-vertexai/opentelemetry/instrumentation/vertexai/event_emitter.py (1)
64-64: Consider renamingevent_loggerparameter for clarity.The parameter is still named
event_loggerthroughout the file, but after the migration it's now aLogger(not anEventLogger). While this maintains consistency with the existing code, renaming it tologgerwould better reflect its current type and purpose.Additionally, consider adding type hints to clarify the expected type:
from opentelemetry.sdk._logs import Logger def emit_prompt_events(args, logger: Logger): ... def emit_response_events(response, logger: Logger): ... def emit_event(event: Union[MessageEvent, ChoiceEvent], logger: Logger) -> None: ...Also applies to: 76-76, 101-101, 119-119, 151-151
packages/opentelemetry-instrumentation-llamaindex/opentelemetry/instrumentation/llamaindex/event_emitter.py (1)
11-11: LogRecord-based emission is correct; consider emitting viaLogger.emitkwargs for future-proofingThe migration from
EventtoLogRecordwithevent_namepreserves the original semantics and respectsshould_emit_events/should_send_prompts. To reduce coupling to theLogRecordconstructor, you can let the SDK build the record by passing the payload directly toLogger.emit, which is designed to acceptbody,attributes, andevent_namekeyword arguments.A small refactor would look like:
def _emit_message_event(event: MessageEvent) -> None: @@ - log_record = LogRecord( - body=body, - attributes=EVENT_ATTRIBUTES, - event_name=name - ) - Config.event_logger.emit(log_record) + Config.event_logger.emit( + body=body, + attributes=EVENT_ATTRIBUTES, + event_name=name, + ) @@ def _emit_choice_event(event: ChoiceEvent) -> None: @@ - log_record = LogRecord( - body=body, - attributes=EVENT_ATTRIBUTES, - event_name="gen_ai.choice" - ) - Config.event_logger.emit(log_record) + Config.event_logger.emit( + body=body, + attributes=EVENT_ATTRIBUTES, + event_name="gen_ai.choice", + )Please double-check this against the exact
Logger.emitsignature for the pinned OpenTelemetry version (1.38.0) before changing, in case additional parameters or behavior need to be preserved.Also applies to: 108-138, 140-161
packages/opentelemetry-instrumentation-mistralai/opentelemetry/instrumentation/mistralai/event_emitter.py (1)
5-5: LogRecord-based emitters are API-compatible; optional robustness tweak availableVerification confirms that Logger.emit(record: LogRecord) is supported and LogRecord constructor accepts body, attributes, and event_name as keyword arguments in OpenTelemetry Python 1.38.0, so the code's use of these APIs is correct and remains compatible.
The refactor from Events to
Logger/LogRecordis sound:
emit_eventcorrectly guards onshould_emit_events()and presence of aLogger, then dispatches to handlers.- Both
_emit_message_eventand_emit_choice_eventbuildLogRecordinstances with the body shaped to match tests (including prompt redaction whenshould_send_prompts()is False),attributes=EVENT_ATTRIBUTESto pinGEN_AI_SYSTEMto"mistral_ai", andevent_nameset appropriately.For slightly more defensive code against future dataclass changes, you could replace
del body["content"]withbody.pop("content", None)to avoidKeyErrorif those fields become optional, but this is optional and not required for current behavior.packages/opentelemetry-instrumentation-cohere/opentelemetry/instrumentation/cohere/__init__.py (1)
316-319: Add space around assignment operator.The logger initialization logic is correct, but Line 316 has a formatting inconsistency:
logger_provider=should have a space after the=operator.Apply this diff to fix the formatting:
- logger_provider= kwargs.get("logger_provider") + logger_provider = kwargs.get("logger_provider")packages/opentelemetry-instrumentation-vertexai/opentelemetry/instrumentation/vertexai/__init__.py (1)
340-345: Add space around assignment operator.The logger initialization logic is correct, but Line 340 has a formatting inconsistency:
logger_provider=should have a space after the=operator.Apply this diff to fix the formatting:
- logger_provider= kwargs.get("logger_provider") + logger_provider = kwargs.get("logger_provider")packages/opentelemetry-instrumentation-together/opentelemetry/instrumentation/together/__init__.py (1)
153-156: Add space around assignment operator.The logger initialization logic is correct, but Line 153 has a formatting inconsistency:
logger_provider=should have a space after the=operator.Apply this diff to fix the formatting:
- logger_provider= kwargs.get("logger_provider") + logger_provider = kwargs.get("logger_provider")packages/opentelemetry-instrumentation-alephalpha/opentelemetry/instrumentation/alephalpha/event_emitter.py (1)
4-4: Aleph Alpha LogRecord migration looks good; add a small guard for missing loggerThe switch to
LogRecord(body=..., attributes=EVENT_ATTRIBUTES, event_name=...)andevent_logger.emit(log_record)matches the expected API and preserves the previous field mapping.To avoid accidental
AttributeErrorifemit_eventis ever called withevent_logger=None(e.g., from tests or misconfigured instrumentors), consider short‑circuiting when no logger is available:def emit_event(event: Union[PromptEvent, CompletionEvent], event_logger) -> None: from opentelemetry.instrumentation.alephalpha import ( should_emit_events, ) @@ - if not should_emit_events(): + if not should_emit_events() or event_logger is None: returnThis keeps behavior unchanged in the normal path while making the helper more robust to misuse.
Also applies to: 28-40, 82-87, 116-122
packages/opentelemetry-instrumentation-together/opentelemetry/instrumentation/together/event_emitter.py (1)
5-5: Together emitter: Logger/LogRecord migration is correct; align logger typing and add None guardThe move to constructing
LogRecordinstances and emitting them viaLogger.emitis consistent and keeps the previous semantics.Given
emit_eventadvertisesevent_logger: Union[Logger, None]but_emit_message_event/_emit_choice_eventrequire a concreteLogger, it’s safer to either (a) treat the logger as non‑optional everywhere, or (b) guard againstNoneat the entry point. Option (b) is minimally invasive:-def emit_event( - event: Union[PromptEvent, CompletionEvent], event_logger: Union[Logger, None] -) -> None: +def emit_event( + event: Union[PromptEvent, CompletionEvent], event_logger: Union[Logger, None] +) -> None: @@ - if not should_emit_events(): + if not should_emit_events() or event_logger is None: returnYou may also simplify the helper type hints to
Optional[Logger]to reflect this more clearly.Also applies to: 95-111, 115-145, 147-169
packages/opentelemetry-instrumentation-google-generativeai/opentelemetry/instrumentation/google_generativeai/event_emitter.py (1)
6-7: Google Generative AI emitter: LogRecord migration is correct; clarify/guard logger usageThe conversion to
LogRecordwithbody,EVENT_ATTRIBUTES, andevent_nameand emission throughLogger.emitis consistent and keeps prior semantics for both message and choice events.Two small cleanups would make this safer and clearer:
- Treat
event_loggeras optional and guard at the entry point:-def emit_event( - event: Union[MessageEvent, ChoiceEvent], event_logger: Union[Logger] -) -> None: +from typing import Optional + +def emit_event( + event: Union[MessageEvent, ChoiceEvent], event_logger: Optional[Logger] +) -> None: @@ - if not should_emit_events(): + if not should_emit_events() or event_logger is None: return
- For consistency, update the other helpers’ annotations to match:
-def emit_message_events(args, kwargs, event_logger: Union[Logger]): +def emit_message_events(args, kwargs, event_logger: Optional[Logger]): @@ -def emit_choice_events( - response: GenerateContentResponse, event_logger: Union[Logger] -): +def emit_choice_events( + response: GenerateContentResponse, event_logger: Optional[Logger] +): @@ -def _emit_message_event(event: MessageEvent, event_logger: Union[Logger]) -> None: +def _emit_message_event(event: MessageEvent, event_logger: Logger) -> None: @@ -def _emit_choice_event(event: ChoiceEvent, event_logger: Union[Logger]) -> None: +def _emit_choice_event(event: ChoiceEvent, event_logger: Logger) -> None:This keeps the internal helpers assuming a concrete
Loggerwhile making the public API resilient to missing loggers.Also applies to: 35-55, 58-76, 94-125, 127-150
packages/opentelemetry-instrumentation-anthropic/tests/test_messages.py (1)
2445-2456: Event name migration is correct; consider tightening the matcherUsing
log.log_record.event_nameis the right adaptation for LogRecord-based logging. However, this helper uses two separateany(...)checks: one forevent_nameand one forbody, which means it doesn’t guarantee that the same log entry matches both conditions. Consider combining them into a single predicate over the logs so each call asserts there is one log with both the expectedevent_nameandbody.If you decide to refactor, rerun the anthropic message tests to ensure no expectations rely on the current, more permissive behavior.
packages/opentelemetry-instrumentation-langchain/tests/test_agents.py (1)
248-255: LangChain agent log helper: good event_name migration; assertion can be stricterReading the event name via
log.log_record.event_nameis the right move for LogRecord-based logging. As with similar helpers, you may want to strengthen this by asserting over a singleany(...)that matches bothevent_nameandexpected_contenton the same log, instead of two independentany(...)checks.If you tighten the predicate, rerun the langchain agent tests to confirm that no assumptions were relying on the previous looser matching.
packages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/__init__.py (1)
9-9: Remove unusedLoggerimport.The
Loggertype is imported but not referenced anywhere in this file.Apply this diff:
-from opentelemetry._logs import Logger, get_logger +from opentelemetry._logs import get_loggerpackages/opentelemetry-instrumentation-bedrock/opentelemetry/instrumentation/bedrock/__init__.py (1)
11-11: Remove unusedLoggerimport.The
Loggertype is imported but not referenced anywhere in this file.Apply this diff:
-from opentelemetry._logs import Logger, get_logger +from opentelemetry._logs import get_loggerpackages/opentelemetry-instrumentation-bedrock/opentelemetry/instrumentation/bedrock/event_emitter.py (1)
296-302: Consider removing trailing newline for consistency.The
event_nameparameter on line 300 has an extra blank line before the closing parenthesis. While not incorrect, removing it would improve consistency with the other LogRecord construction at lines 272-277.Apply this diff:
log_record = LogRecord( body=body, attributes=EVENT_ATTRIBUTES, event_name="gen_ai.choice" - )packages/opentelemetry-instrumentation-vertexai/tests/conftest.py (1)
82-97: Remove unusedreaderparameter.Both
instrument_with_contentandinstrument_with_no_contentfixtures declare areaderparameter that is never used. Consider removing it to clean up the function signatures.Apply this diff:
@pytest.fixture(scope="function") def instrument_with_content( - reader, tracer_provider, logger_provider, meter_provider + tracer_provider, logger_provider, meter_provider ):@pytest.fixture(scope="function") def instrument_with_no_content( - reader, tracer_provider, logger_provider, meter_provider + tracer_provider, logger_provider, meter_provider ):Also applies to: 100-116
packages/opentelemetry-instrumentation-bedrock/tests/conftest.py (1)
102-120: Remove unusedreaderparameter.Both
instrument_with_contentandinstrument_with_no_contentfixtures declare areaderparameter that is never used. Consider removing it to clean up the function signatures.Apply this diff:
@pytest.fixture(scope="function") def instrument_with_content( - reader, tracer_provider, logger_provider, meter_provider + tracer_provider, logger_provider, meter_provider ):@pytest.fixture(scope="function") def instrument_with_no_content( - reader, tracer_provider, logger_provider, meter_provider + tracer_provider, logger_provider, meter_provider ):Also applies to: 123-141
packages/opentelemetry-instrumentation-langchain/tests/conftest.py (1)
6-6: Remove unusedLoggerimport.The
Loggertype is imported but not referenced anywhere in this file. Onlyget_loggeris used.Apply this diff:
-from opentelemetry._logs import Logger, get_logger +from opentelemetry._logs import get_loggerpackages/opentelemetry-instrumentation-groq/opentelemetry/instrumentation/groq/event_emitter.py (1)
146-152: Minor: Remove trailing whitespace.Line 150 has trailing whitespace after the event_name assignment.
Apply this diff:
log_record = LogRecord( body=body, attributes=EVENT_ATTRIBUTES, event_name="gen_ai.choice" - )packages/opentelemetry-instrumentation-openai/tests/conftest.py (1)
7-7: Remove unused import.The
Loggerimport is not used in this file. Onlyget_loggeris needed for obtaining the logger instance.Apply this diff:
-from opentelemetry._logs import Logger, get_logger +from opentelemetry._logs import get_logger
...pentelemetry-instrumentation-alephalpha/opentelemetry/instrumentation/alephalpha/__init__.py
Outdated
Show resolved
Hide resolved
...ages/opentelemetry-instrumentation-bedrock/opentelemetry/instrumentation/bedrock/__init__.py
Outdated
Show resolved
Hide resolved
| elif headers and headers.get("x-amzn-bedrock-input-token-count") is not None: | ||
| # For Anthropic V2 models (claude-v2), token counts are in HTTP headers | ||
| input_tokens = int(headers.get("x-amzn-bedrock-input-token-count", 0)) | ||
| output_tokens = int(headers.get("x-amzn-bedrock-output-token-count", 0)) | ||
| _record_usage_to_span( | ||
| span, | ||
| input_tokens, | ||
| output_tokens, | ||
| metric_params, | ||
| ) |
There was a problem hiding this comment.
Consider error handling for invalid header values.
The code assumes header values are valid integer strings when present. If AWS returns malformed headers, int() will raise ValueError.
Consider wrapping the conversion in a try-except block:
elif headers and headers.get("x-amzn-bedrock-input-token-count") is not None:
# For Anthropic V2 models (claude-v2), token counts are in HTTP headers
- input_tokens = int(headers.get("x-amzn-bedrock-input-token-count", 0))
- output_tokens = int(headers.get("x-amzn-bedrock-output-token-count", 0))
- _record_usage_to_span(
- span,
- input_tokens,
- output_tokens,
- metric_params,
- )
+ try:
+ input_tokens = int(headers.get("x-amzn-bedrock-input-token-count", 0))
+ output_tokens = int(headers.get("x-amzn-bedrock-output-token-count", 0))
+ _record_usage_to_span(
+ span,
+ input_tokens,
+ output_tokens,
+ metric_params,
+ )
+ except (ValueError, TypeError):
+ pass # Skip token recording if headers contain invalid values📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| elif headers and headers.get("x-amzn-bedrock-input-token-count") is not None: | |
| # For Anthropic V2 models (claude-v2), token counts are in HTTP headers | |
| input_tokens = int(headers.get("x-amzn-bedrock-input-token-count", 0)) | |
| output_tokens = int(headers.get("x-amzn-bedrock-output-token-count", 0)) | |
| _record_usage_to_span( | |
| span, | |
| input_tokens, | |
| output_tokens, | |
| metric_params, | |
| ) | |
| elif headers and headers.get("x-amzn-bedrock-input-token-count") is not None: | |
| # For Anthropic V2 models (claude-v2), token counts are in HTTP headers | |
| try: | |
| input_tokens = int(headers.get("x-amzn-bedrock-input-token-count", 0)) | |
| output_tokens = int(headers.get("x-amzn-bedrock-output-token-count", 0)) | |
| _record_usage_to_span( | |
| span, | |
| input_tokens, | |
| output_tokens, | |
| metric_params, | |
| ) | |
| except (ValueError, TypeError): | |
| pass # Skip token recording if headers contain invalid values |
🤖 Prompt for AI Agents
In
packages/opentelemetry-instrumentation-bedrock/opentelemetry/instrumentation/bedrock/span_utils.py
around lines 230 to 239, the code directly casts header values to int which will
raise ValueError for malformed headers; catch exceptions when converting
x-amzn-bedrock-input-token-count and x-amzn-bedrock-output-token-count to int,
default the token counts to 0 on failure, and record or log the parsing error
(e.g., attach an error attribute to the span or use the module logger) before
calling _record_usage_to_span so malformed header values do not crash the
instrumentation.
| elif headers and headers.get("x-amzn-bedrock-input-token-count") is not None: | ||
| # For Anthropic V2 models (claude-v2), token counts are in HTTP headers | ||
| prompt_tokens = int(headers.get("x-amzn-bedrock-input-token-count", 0)) | ||
| completion_tokens = int(headers.get("x-amzn-bedrock-output-token-count", 0)) | ||
| _record_usage_to_span(span, prompt_tokens, completion_tokens, metric_params) |
There was a problem hiding this comment.
Consider error handling for invalid header values.
Same issue as in the completion path: the code assumes header values are valid integer strings when present.
Consider wrapping the conversion in a try-except block:
elif headers and headers.get("x-amzn-bedrock-input-token-count") is not None:
# For Anthropic V2 models (claude-v2), token counts are in HTTP headers
- prompt_tokens = int(headers.get("x-amzn-bedrock-input-token-count", 0))
- completion_tokens = int(headers.get("x-amzn-bedrock-output-token-count", 0))
- _record_usage_to_span(span, prompt_tokens, completion_tokens, metric_params)
+ try:
+ prompt_tokens = int(headers.get("x-amzn-bedrock-input-token-count", 0))
+ completion_tokens = int(headers.get("x-amzn-bedrock-output-token-count", 0))
+ _record_usage_to_span(span, prompt_tokens, completion_tokens, metric_params)
+ except (ValueError, TypeError):
+ pass # Skip token recording if headers contain invalid values🤖 Prompt for AI Agents
In
packages/opentelemetry-instrumentation-bedrock/opentelemetry/instrumentation/bedrock/span_utils.py
around lines 301 to 305, the code parses header token counts with int(...)
without guarding against non-integer values; wrap the two int(...) conversions
in a try/except ValueError (and TypeError) block, defaulting prompt_tokens and
completion_tokens to 0 on parse failure, and optionally record a warning (e.g.,
on the span or logger) noting the malformed header before calling
_record_usage_to_span(span, prompt_tokens, completion_tokens, metric_params).
...umentation-google-generativeai/opentelemetry/instrumentation/google_generativeai/__init__.py
Outdated
Show resolved
Hide resolved
.../opentelemetry-instrumentation-sagemaker/opentelemetry/instrumentation/sagemaker/__init__.py
Outdated
Show resolved
Hide resolved
.../opentelemetry-instrumentation-sagemaker/opentelemetry/instrumentation/sagemaker/__init__.py
Outdated
Show resolved
Hide resolved
...elemetry-instrumentation-transformers/opentelemetry/instrumentation/transformers/__init__.py
Outdated
Show resolved
Hide resolved
...elemetry-instrumentation-transformers/opentelemetry/instrumentation/transformers/__init__.py
Outdated
Show resolved
Hide resolved
...entelemetry-instrumentation-vertexai/opentelemetry/instrumentation/vertexai/event_emitter.py
Show resolved
Hide resolved
galkleinman
changed the title
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
packages/opentelemetry-instrumentation-together/opentelemetry/instrumentation/together/event_emitter.py (1)
95-112: Add None check for event_logger to prevent potential AttributeError.The type annotation on Line 96 allows
event_loggerto beNone, but lines 108 and 110 call methods on it without checking forNonefirst. This will cause anAttributeErrorat runtime ifNoneis passed.Apply this diff to add a None check:
def emit_event( event: Union[PromptEvent, CompletionEvent], event_logger: Union[Logger, None] ) -> None: """ Emit an event to the OpenTelemetry SDK. Args: event: The event to emit. """ - if not should_emit_events(): + if not should_emit_events() or event_logger is None: return if isinstance(event, PromptEvent): _emit_message_event(event, event_logger) elif isinstance(event, CompletionEvent): _emit_choice_event(event, event_logger) else: raise TypeError("Unsupported event type")packages/opentelemetry-instrumentation-watsonx/opentelemetry/instrumentation/watsonx/__init__.py (2)
489-500: _handle_input signature vs call mismatch will raise a TypeError
_handle_inputis defined with 7 parameters afterspan:def _handle_input(span, event_logger, name, instance, response_counter, args, kwargs):but is called as:
_handle_input(span, event_logger, name, instance, args, kwargs)This shifts
argsintoresponse_counter,kwargsintoargs, and leaveskwargsmissing, which will result in aTypeErrorat runtime when instrumentation executes.A minimal, low-risk fix is to pass
response_counterat the call site:- _handle_input(span, event_logger, name, instance, args, kwargs) + _handle_input(span, event_logger, name, instance, response_counter, args, kwargs)This aligns the call with the existing signature and preserves the pattern used in other instrumentations (even if
response_counteris currently unused inside_handle_input).Also applies to: 581-581
362-370: _emit_response_events does not accept or forward event_logger, breaking emit_event usageThere are two related issues around response event emission:
_emit_response_eventsdoes not acceptevent_loggerand does not pass it toemit_event:def _emit_response_events(response: dict): for i, message in enumerate(response.get("results", [])): emit_event( ChoiceEvent(...), )However,
emit_eventnow requires anevent_loggerargument, so this call will fail or use an incorrect signature.
- Call sites are inconsistent with the current
_emit_response_eventssignature:
- In
_handle_response:if should_emit_events() and event_logger: _emit_response_events(responses, event_logger)
- In
_handle_stream_response:if should_emit_events() and event_logger: _emit_response_events( { "results": [ { "stop_reason": stream_stop_reason, "generated_text": stream_generated_text, } ] }, )The first call passes an extra positional argument (raising
TypeErrorunder the current definition), and the second call omitsevent_logger, so even if the signature were updated, it would still not propagate the logger in the streaming path.To align with the new
emit_eventAPI and avoid runtime errors, update_emit_response_eventsto acceptevent_loggerand forward it, and update the streaming call to pass it:-def _emit_response_events(response: dict): +def _emit_response_events(response: dict, event_logger): for i, message in enumerate(response.get("results", [])): emit_event( ChoiceEvent( index=i, message={"content": message.get("generated_text"), "role": "assistant"}, finish_reason=message.get("stop_reason", "unknown"), - ) + ), + event_logger, )Then, in
_handle_stream_response, passevent_logger:- if should_emit_events() and event_logger: - _emit_response_events( + if should_emit_events() and event_logger: + _emit_response_events( { "results": [ { "stop_reason": stream_stop_reason, "generated_text": stream_generated_text, } ] - }, + }, + event_logger, )With these changes, both non-streaming and streaming response paths will correctly use the
Logger-basedemit_eventwithout raisingTypeError.Also applies to: 503-518, 530-545
🧹 Nitpick comments (19)
packages/opentelemetry-instrumentation-writer/tests/test_completions.py (1)
346-347: Remove duplicate assertion.Line 346 and 347 contain identical assertions. Remove the duplicate on line 347 for cleaner code.
Apply this diff:
assert writer_span.name == "writerai.completions" - assert writer_span.name == "writerai.completions" assert writer_span.attributes.get(f"{SpanAttributes.LLM_SYSTEM}") == "writer"packages/opentelemetry-instrumentation-google-generativeai/opentelemetry/instrumentation/google_generativeai/event_emitter.py (1)
144-150: Remove extraneous blank line for consistency.There's an unnecessary blank line at line 148 inside the LogRecord constructor call.
Apply this diff to clean up the formatting:
log_record = LogRecord( body=body, attributes=EVENT_ATTRIBUTES, event_name="gen_ai.choice" - )packages/opentelemetry-instrumentation-together/opentelemetry/instrumentation/together/event_emitter.py (1)
147-169: LGTM! LogRecord emission pattern is correct.The migration to LogRecord with
event_nameis properly implemented. Minor style note: Line 167 has an extra blank line inside the LogRecord constructor that could be removed for consistency.Optional formatting cleanup:
log_record = LogRecord( body=body, attributes=EVENT_ATTRIBUTES, - event_name="gen_ai.choice" - + event_name="gen_ai.choice" ) event_logger.emit(log_record)packages/opentelemetry-instrumentation-openai/tests/conftest.py (2)
153-154: Consider removing redundant fixture parameters.The parameters
reader,tracer_provider, andmeter_providerare not used in this function and are already declared as dependencies ofinstrument_legacy(line 134). Pytest will automatically initialize them through the dependency chain, making their explicit declaration here redundant.Apply this diff to simplify the fixture signature:
@pytest.fixture(scope="function") def instrument_with_content( - instrument_legacy, reader, tracer_provider, logger_provider, meter_provider + instrument_legacy, logger_provider ):
173-174: Consider removing redundant fixture parameters.Similar to
instrument_with_content, the parametersreader,tracer_provider, andmeter_providerare redundant since they're already dependencies ofinstrument_legacy(line 134).Apply this diff to simplify the fixture signature:
@pytest.fixture(scope="function") def instrument_with_no_content( - instrument_legacy, reader, tracer_provider, logger_provider, meter_provider + instrument_legacy, logger_provider ):packages/opentelemetry-instrumentation-replicate/opentelemetry/instrumentation/replicate/event_emitter.py (1)
137-143: Remove blank line before closing parenthesis.The LogRecord emission logic is correct, but there's an unnecessary blank line at line 141 between the
event_nameparameter and the closing parenthesis.Apply this diff to improve formatting:
log_record = LogRecord( body=body, attributes=EVENT_ATTRIBUTES, event_name="gen_ai.choice" - )packages/opentelemetry-instrumentation-vertexai/opentelemetry/instrumentation/vertexai/__init__.py (1)
340-345: Optional refactoring: Renameevent_loggertologgerfor clarityVerification confirms the migration to
get_loggeris correct and fully compatible. The Logger API's.emit()method matches the requirements inevent_emitter.pyfunctions (emit_prompt_events,emit_response_events, and the internal_emit_message_event,_emit_choice_event). All 18+ usages throughout the wrapper functions are compatible.The variable name
event_loggerremains misleading since it holds a standardLoggerinstance rather than anEventLogger. Renaming tologgerwould improve code clarity for maintainers:- event_logger = None + logger = None if should_emit_events(): logger_provider = kwargs.get("logger_provider") - event_logger = get_logger( + logger = get_logger( __name__, __version__, logger_provider=logger_provider, )Then update all function parameter names and references (18 occurrences throughout the file) from
event_loggertologger.packages/opentelemetry-instrumentation-groq/opentelemetry/instrumentation/groq/event_emitter.py (1)
146-152: Minor formatting inconsistency.The LogRecord construction is correct, but there's an unnecessary blank line (line 150) between the
event_nameparameter and the closing parenthesis. For consistency with the message event construction (lines 122-127), consider removing this blank line.Apply this diff to improve formatting consistency:
log_record = LogRecord( body=body, attributes=EVENT_ATTRIBUTES, event_name="gen_ai.choice" - )packages/opentelemetry-instrumentation-ollama/opentelemetry/instrumentation/ollama/event_emitter.py (2)
187-193: Minor formatting inconsistency with extra blank line.The LogRecord construction has an extra blank line at Line 191, which is inconsistent with the formatting in
_emit_message_event(lines 163-167). While not a functional issue, removing it would improve code consistency.Apply this diff to align the formatting:
log_record = LogRecord( body=body, attributes=EVENT_ATTRIBUTES, event_name="gen_ai.choice" - )
121-121: Consider adding type hints for the event_logger parameter.The
event_loggerparameter lacks type annotations throughout the file (inemit_event,_emit_message_event,_emit_choice_event, and other functions). Adding type hints would clarify the expected interface after the migration from EventLogger to Logger.For example, you could add type hints like:
from opentelemetry.sdk._logs import Logger def emit_event(event: Union[MessageEvent, ChoiceEvent], event_logger: Logger) -> None: # ...This would improve IDE support and make the migration's impact on the API more explicit.
packages/opentelemetry-instrumentation-transformers/opentelemetry/instrumentation/transformers/event_emitter.py (2)
143-149: Clean up formatting: remove blank line inside constructor call.There's an unnecessary blank line after the
event_nameparameter on line 147, which breaks the visual flow of the constructor call.Apply this diff to fix the formatting:
log_record = LogRecord( body=body, attributes=EVENT_ATTRIBUTES, event_name="gen_ai.choice" - ) event_logger.emit(log_record)
34-34: Consider renamingevent_loggerparameter tologgerfor API clarity.The parameter name
event_loggeris a holdover from the EventLogger API and is now misleading since the code uses the Logger API. Renaming it tologgerthroughout the file would improve clarity and align with the new LogRecord-based API.This affects the following functions:
emit_prompt_events(line 34)emit_response_events(line 51)emit_event(line 67)_emit_prompt_event(line 85)_emit_completion_event(line 121)Example for
_emit_prompt_event:-def _emit_prompt_event(event: PromptEvent, event_logger) -> None: +def _emit_prompt_event(event: PromptEvent, logger) -> None: body = { content": event.content, "role": event.role, "tool_calls": event.tool_calls, } # ... rest of function ... - event_logger.emit(log_record) + logger.emit(log_record)Also applies to: 51-51, 67-67, 85-85, 121-121
packages/opentelemetry-instrumentation-alephalpha/opentelemetry/instrumentation/alephalpha/__init__.py (2)
41-41: Confirm intent behind removing the upper bound onaleph_alpha_clientRelaxing
_instrumentsto only a lower bound (>= 7.1.0) allows futurealeph_alpha_clientmajor versions to be used, which can introduce breaking API changes that this instrumentation may not be ready for. If you haven’t already, consider explicitly validating against the latest major(s) and, if needed, re‑introducing a conservative upper bound (e.g.,<9.0.0) once compatibility is known.
141-149: Unify optional logger typing for minor clarity
event_loggeris typed asUnion[Logger, None]here and asOptional[Logger]in_handle_message_event. They’re equivalent, but for readability and type‑checker friendliness, consider standardizing onOptional[Logger]across this file.packages/opentelemetry-instrumentation-bedrock/opentelemetry/instrumentation/bedrock/event_emitter.py (1)
6-6: LogRecord migration looks good; consider guardingevent_loggerwhen typed as OptionalThe switch to
Logger/LogRecordand settingevent_name(gen_ai.*) plusEVENT_ATTRIBUTESis consistent with the other instrumentations and matches the tests’ expectations.Given the signatures use
Optional[Logger](e.g., Lines 33, 64, 210, 227, 247, 280) but_emit_message_eventand_emit_choice_eventdereferenceevent_loggerwithout a None check (Lines 277, 302), a defensive guard would make this helper safer if it’s ever called without theevent_loggertruthiness checks used inbedrock.__init__.For example:
def _emit_message_event( - event: MessageEvent, event_logger: Optional[Logger] + event: MessageEvent, event_logger: Optional[Logger] ) -> None: + if event_logger is None: + return @@ - log_record = LogRecord( - body=body, - attributes=EVENT_ATTRIBUTES, - event_name=name - ) - event_logger.emit(log_record) + log_record = LogRecord( + body=body, + attributes=EVENT_ATTRIBUTES, + event_name=name, + ) + event_logger.emit(log_record) @@ -def _emit_choice_event(event: ChoiceEvent, event_logger: Optional[Logger]) -> None: +def _emit_choice_event(event: ChoiceEvent, event_logger: Optional[Logger]) -> None: + if event_logger is None: + returnNot required for correctness today, but it aligns the implementation with the type hints and prevents accidental
NoneType.emiterrors.Also applies to: 33-64, 209-223, 226-228, 246-248, 272-277, 280-302
packages/opentelemetry-instrumentation-anthropic/opentelemetry/instrumentation/anthropic/event_emitter.py (1)
6-6: Logger/LogRecord migration is correct; logger Optionality could be cleaned upThe Anthropic emitter now mirrors the Cohere pattern:
LogRecordbodies are built from the Message/Choice dataclasses,event_nameis set togen_ai.{role}.message/gen_ai.choice, andEVENT_ATTRIBUTESpins the Anthropic system — this matches the tests’ expectations and the semconv comments.One minor inconsistency:
emit_input_events,emit_response_events, andemit_streaming_response_eventsacceptOptional[Logger], butemit_event,_emit_message_event, and_emit_choice_eventare annotated withLoggerand unconditionally callevent_logger.emit(...). Current instrumentors only call these when a logger is configured, so it’s not a bug, but you may want to either:
- Make all these parameters consistently
Optional[Logger]and early-return whenevent_logger is None, or- Make them all
Loggerand rely on the existingshould_emit_events() and event_loggerguards at call sites.Either approach would make the API a bit clearer for future callers and static analysis.
Also applies to: 37-61, 127-129, 167-169, 187-217, 219-241
packages/opentelemetry-instrumentation-cohere/opentelemetry/instrumentation/cohere/event_emitter.py (1)
147-168: Clarify_parse_response_eventcondition to avoid always-true expressionThis condition:
elif ( llm_request_type == LLMRequestTypeValues.CHAT or LLMRequestTypeValues.COMPLETION ):is effectively always true (because
LLMRequestTypeValues.COMPLETIONis truthy), so every non-RERANK request type will hit this branch. If the intention was to handle only CHAT and COMPLETION here, consider rewriting explicitly for readability and to avoid accidental behavior changes later:- elif ( - llm_request_type == LLMRequestTypeValues.CHAT or LLMRequestTypeValues.COMPLETION - ): + elif llm_request_type in ( + LLMRequestTypeValues.CHAT, + LLMRequestTypeValues.COMPLETION, + ):(or another form that clearly expresses the intended set of handled types).
packages/opentelemetry-instrumentation-writer/opentelemetry/instrumentation/writer/event_emitter.py (1)
104-127: Refinetool_callsfiltering to avoid emittingtool_calls=NoneFor assistant messages without tool calls, this logic will currently leave
tool_calls: Nonein the payload. To better match the intent (“only assistant messages with tool calls should carry the field”) and to be consistent with the Anthropic emitter pattern, you can tighten the condition:- if event.role != Roles.ASSISTANT.value: - body.pop("tool_calls", None) + # Only assistant messages with actual tool calls should include `tool_calls` + if event.role != Roles.ASSISTANT.value or event.tool_calls is None: + body.pop("tool_calls", None)packages/opentelemetry-instrumentation-writer/opentelemetry/instrumentation/writer/__init__.py (1)
604-609: Consider supporting legacyevent_logger_providerkwarg for compatibilitySwitching to
logger_provider = kwargs.get("logger_provider")is good for consistency with other instrumentations, but callers that still passevent_logger_providerwill now be ignored and fall back to the global provider. To keep this migration backwards-compatible, you could accept both:- event_logger = None - if not Config.use_legacy_attributes: - logger_provider = kwargs.get("logger_provider") - event_logger = get_logger( - __name__, __version__, logger_provider=logger_provider - ) + event_logger = None + if not Config.use_legacy_attributes: + # Prefer the new kwarg name but still honor the legacy one. + logger_provider = kwargs.get("logger_provider") or kwargs.get( + "event_logger_provider" + ) + event_logger = get_logger( + __name__, __version__, logger_provider=logger_provider + )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (84)
packages/opentelemetry-instrumentation-alephalpha/opentelemetry/instrumentation/alephalpha/__init__.py(5 hunks)packages/opentelemetry-instrumentation-alephalpha/opentelemetry/instrumentation/alephalpha/event_emitter.py(3 hunks)packages/opentelemetry-instrumentation-alephalpha/tests/test_completion.py(1 hunks)packages/opentelemetry-instrumentation-anthropic/opentelemetry/instrumentation/anthropic/event_emitter.py(8 hunks)packages/opentelemetry-instrumentation-anthropic/tests/test_completion.py(1 hunks)packages/opentelemetry-instrumentation-anthropic/tests/test_messages.py(1 hunks)packages/opentelemetry-instrumentation-anthropic/tests/test_prompt_caching.py(1 hunks)packages/opentelemetry-instrumentation-anthropic/tests/test_thinking.py(1 hunks)packages/opentelemetry-instrumentation-bedrock/opentelemetry/instrumentation/bedrock/__init__.py(2 hunks)packages/opentelemetry-instrumentation-bedrock/opentelemetry/instrumentation/bedrock/event_emitter.py(8 hunks)packages/opentelemetry-instrumentation-bedrock/tests/traces/test_ai21.py(1 hunks)packages/opentelemetry-instrumentation-bedrock/tests/traces/test_anthropic.py(4 hunks)packages/opentelemetry-instrumentation-bedrock/tests/traces/test_cohere.py(1 hunks)packages/opentelemetry-instrumentation-bedrock/tests/traces/test_imported_model.py(1 hunks)packages/opentelemetry-instrumentation-bedrock/tests/traces/test_meta.py(1 hunks)packages/opentelemetry-instrumentation-bedrock/tests/traces/test_nova.py(1 hunks)packages/opentelemetry-instrumentation-bedrock/tests/traces/test_titan.py(1 hunks)packages/opentelemetry-instrumentation-cohere/opentelemetry/instrumentation/cohere/__init__.py(4 hunks)packages/opentelemetry-instrumentation-cohere/opentelemetry/instrumentation/cohere/event_emitter.py(5 hunks)packages/opentelemetry-instrumentation-cohere/tests/test_chat.py(1 hunks)packages/opentelemetry-instrumentation-cohere/tests/test_completion.py(1 hunks)packages/opentelemetry-instrumentation-cohere/tests/test_rerank.py(1 hunks)packages/opentelemetry-instrumentation-google-generativeai/opentelemetry/instrumentation/google_generativeai/__init__.py(2 hunks)packages/opentelemetry-instrumentation-google-generativeai/opentelemetry/instrumentation/google_generativeai/event_emitter.py(7 hunks)packages/opentelemetry-instrumentation-google-generativeai/tests/test_generate_content.py(1 hunks)packages/opentelemetry-instrumentation-groq/opentelemetry/instrumentation/groq/__init__.py(4 hunks)packages/opentelemetry-instrumentation-groq/opentelemetry/instrumentation/groq/event_emitter.py(5 hunks)packages/opentelemetry-instrumentation-groq/tests/traces/test_chat_tracing.py(1 hunks)packages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/__init__.py(2 hunks)packages/opentelemetry-instrumentation-langchain/tests/conftest.py(4 hunks)packages/opentelemetry-instrumentation-langchain/tests/test_agents.py(1 hunks)packages/opentelemetry-instrumentation-langchain/tests/test_chains.py(1 hunks)packages/opentelemetry-instrumentation-langchain/tests/test_documents_chains.py(1 hunks)packages/opentelemetry-instrumentation-langchain/tests/test_lcel.py(1 hunks)packages/opentelemetry-instrumentation-langchain/tests/test_structured_output.py(1 hunks)packages/opentelemetry-instrumentation-langchain/tests/test_tool_calls.py(1 hunks)packages/opentelemetry-instrumentation-llamaindex/opentelemetry/instrumentation/llamaindex/__init__.py(2 hunks)packages/opentelemetry-instrumentation-llamaindex/tests/conftest.py(4 hunks)packages/opentelemetry-instrumentation-llamaindex/tests/test_agents.py(1 hunks)packages/opentelemetry-instrumentation-mistralai/opentelemetry/instrumentation/mistralai/__init__.py(2 hunks)packages/opentelemetry-instrumentation-mistralai/opentelemetry/instrumentation/mistralai/event_emitter.py(5 hunks)packages/opentelemetry-instrumentation-mistralai/tests/test_chat.py(1 hunks)packages/opentelemetry-instrumentation-mistralai/tests/test_embeddings.py(1 hunks)packages/opentelemetry-instrumentation-ollama/opentelemetry/instrumentation/ollama/__init__.py(2 hunks)packages/opentelemetry-instrumentation-ollama/opentelemetry/instrumentation/ollama/event_emitter.py(4 hunks)packages/opentelemetry-instrumentation-ollama/tests/test_chat.py(1 hunks)packages/opentelemetry-instrumentation-ollama/tests/test_embeddings.py(1 hunks)packages/opentelemetry-instrumentation-ollama/tests/test_generation.py(1 hunks)packages/opentelemetry-instrumentation-openai/opentelemetry/instrumentation/openai/v0/__init__.py(2 hunks)packages/opentelemetry-instrumentation-openai/opentelemetry/instrumentation/openai/v1/__init__.py(2 hunks)packages/opentelemetry-instrumentation-openai/tests/conftest.py(4 hunks)packages/opentelemetry-instrumentation-openai/tests/traces/test_assistant.py(1 hunks)packages/opentelemetry-instrumentation-openai/tests/traces/test_azure.py(1 hunks)packages/opentelemetry-instrumentation-openai/tests/traces/test_chat.py(1 hunks)packages/opentelemetry-instrumentation-openai/tests/traces/test_chat_parse.py(1 hunks)packages/opentelemetry-instrumentation-openai/tests/traces/test_completions.py(1 hunks)packages/opentelemetry-instrumentation-openai/tests/traces/test_embeddings.py(1 hunks)packages/opentelemetry-instrumentation-openai/tests/traces/test_functions.py(1 hunks)packages/opentelemetry-instrumentation-openai/tests/traces/test_prompt_caching.py(1 hunks)packages/opentelemetry-instrumentation-openai/tests/traces/test_span_context_propagation.py(1 hunks)packages/opentelemetry-instrumentation-openai/tests/traces/test_vision.py(1 hunks)packages/opentelemetry-instrumentation-replicate/opentelemetry/instrumentation/replicate/__init__.py(2 hunks)packages/opentelemetry-instrumentation-replicate/opentelemetry/instrumentation/replicate/event_emitter.py(6 hunks)packages/opentelemetry-instrumentation-replicate/tests/test_image_generation.py(1 hunks)packages/opentelemetry-instrumentation-replicate/tests/test_llama.py(1 hunks)packages/opentelemetry-instrumentation-sagemaker/opentelemetry/instrumentation/sagemaker/__init__.py(2 hunks)packages/opentelemetry-instrumentation-sagemaker/opentelemetry/instrumentation/sagemaker/event_emitter.py(5 hunks)packages/opentelemetry-instrumentation-sagemaker/tests/test_invocation.py(1 hunks)packages/opentelemetry-instrumentation-together/opentelemetry/instrumentation/together/__init__.py(2 hunks)packages/opentelemetry-instrumentation-together/opentelemetry/instrumentation/together/event_emitter.py(5 hunks)packages/opentelemetry-instrumentation-together/tests/test_chat.py(1 hunks)packages/opentelemetry-instrumentation-together/tests/test_completion.py(1 hunks)packages/opentelemetry-instrumentation-transformers/opentelemetry/instrumentation/transformers/__init__.py(2 hunks)packages/opentelemetry-instrumentation-transformers/opentelemetry/instrumentation/transformers/event_emitter.py(3 hunks)packages/opentelemetry-instrumentation-transformers/tests/test_pipeline.py(1 hunks)packages/opentelemetry-instrumentation-vertexai/opentelemetry/instrumentation/vertexai/__init__.py(2 hunks)packages/opentelemetry-instrumentation-vertexai/opentelemetry/instrumentation/vertexai/event_emitter.py(3 hunks)packages/opentelemetry-instrumentation-watsonx/opentelemetry/instrumentation/watsonx/__init__.py(3 hunks)packages/opentelemetry-instrumentation-watsonx/opentelemetry/instrumentation/watsonx/event_emitter.py(5 hunks)packages/opentelemetry-instrumentation-watsonx/tests/traces/test_generate.py(1 hunks)packages/opentelemetry-instrumentation-writer/opentelemetry/instrumentation/writer/__init__.py(4 hunks)packages/opentelemetry-instrumentation-writer/opentelemetry/instrumentation/writer/event_emitter.py(5 hunks)packages/opentelemetry-instrumentation-writer/tests/test_chat.py(1 hunks)packages/opentelemetry-instrumentation-writer/tests/test_completions.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (41)
- packages/opentelemetry-instrumentation-google-generativeai/tests/test_generate_content.py
- packages/opentelemetry-instrumentation-openai/tests/traces/test_azure.py
- packages/opentelemetry-instrumentation-anthropic/tests/test_prompt_caching.py
- packages/opentelemetry-instrumentation-replicate/tests/test_llama.py
- packages/opentelemetry-instrumentation-alephalpha/tests/test_completion.py
- packages/opentelemetry-instrumentation-openai/tests/traces/test_span_context_propagation.py
- packages/opentelemetry-instrumentation-bedrock/tests/traces/test_nova.py
- packages/opentelemetry-instrumentation-bedrock/tests/traces/test_titan.py
- packages/opentelemetry-instrumentation-sagemaker/tests/test_invocation.py
- packages/opentelemetry-instrumentation-openai/opentelemetry/instrumentation/openai/v0/init.py
- packages/opentelemetry-instrumentation-langchain/tests/test_lcel.py
- packages/opentelemetry-instrumentation-langchain/tests/test_agents.py
- packages/opentelemetry-instrumentation-bedrock/tests/traces/test_ai21.py
- packages/opentelemetry-instrumentation-cohere/tests/test_completion.py
- packages/opentelemetry-instrumentation-openai/tests/traces/test_functions.py
- packages/opentelemetry-instrumentation-llamaindex/opentelemetry/instrumentation/llamaindex/init.py
- packages/opentelemetry-instrumentation-langchain/tests/test_chains.py
- packages/opentelemetry-instrumentation-anthropic/tests/test_messages.py
- packages/opentelemetry-instrumentation-ollama/tests/test_embeddings.py
- packages/opentelemetry-instrumentation-together/tests/test_chat.py
- packages/opentelemetry-instrumentation-bedrock/tests/traces/test_imported_model.py
- packages/opentelemetry-instrumentation-mistralai/tests/test_chat.py
- packages/opentelemetry-instrumentation-anthropic/tests/test_completion.py
- packages/opentelemetry-instrumentation-openai/tests/traces/test_prompt_caching.py
- packages/opentelemetry-instrumentation-openai/tests/traces/test_assistant.py
- packages/opentelemetry-instrumentation-anthropic/tests/test_thinking.py
- packages/opentelemetry-instrumentation-cohere/tests/test_rerank.py
- packages/opentelemetry-instrumentation-openai/tests/traces/test_vision.py
- packages/opentelemetry-instrumentation-langchain/tests/test_tool_calls.py
- packages/opentelemetry-instrumentation-langchain/tests/test_structured_output.py
- packages/opentelemetry-instrumentation-vertexai/opentelemetry/instrumentation/vertexai/event_emitter.py
- packages/opentelemetry-instrumentation-transformers/tests/test_pipeline.py
- packages/opentelemetry-instrumentation-mistralai/opentelemetry/instrumentation/mistralai/event_emitter.py
- packages/opentelemetry-instrumentation-openai/tests/traces/test_chat_parse.py
- packages/opentelemetry-instrumentation-replicate/tests/test_image_generation.py
- packages/opentelemetry-instrumentation-llamaindex/tests/conftest.py
- packages/opentelemetry-instrumentation-alephalpha/opentelemetry/instrumentation/alephalpha/event_emitter.py
- packages/opentelemetry-instrumentation-langchain/tests/test_documents_chains.py
- packages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/init.py
- packages/opentelemetry-instrumentation-writer/tests/test_chat.py
- packages/opentelemetry-instrumentation-together/opentelemetry/instrumentation/together/init.py
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Store API keys only in environment variables/secure vaults; never hardcode secrets in code
Use Flake8 for code linting and adhere to its rules
Files:
packages/opentelemetry-instrumentation-ollama/opentelemetry/instrumentation/ollama/event_emitter.pypackages/opentelemetry-instrumentation-llamaindex/tests/test_agents.pypackages/opentelemetry-instrumentation-watsonx/opentelemetry/instrumentation/watsonx/event_emitter.pypackages/opentelemetry-instrumentation-cohere/tests/test_chat.pypackages/opentelemetry-instrumentation-groq/tests/traces/test_chat_tracing.pypackages/opentelemetry-instrumentation-openai/opentelemetry/instrumentation/openai/v1/__init__.pypackages/opentelemetry-instrumentation-transformers/opentelemetry/instrumentation/transformers/event_emitter.pypackages/opentelemetry-instrumentation-bedrock/opentelemetry/instrumentation/bedrock/event_emitter.pypackages/opentelemetry-instrumentation-alephalpha/opentelemetry/instrumentation/alephalpha/__init__.pypackages/opentelemetry-instrumentation-together/opentelemetry/instrumentation/together/event_emitter.pypackages/opentelemetry-instrumentation-cohere/opentelemetry/instrumentation/cohere/__init__.pypackages/opentelemetry-instrumentation-groq/opentelemetry/instrumentation/groq/__init__.pypackages/opentelemetry-instrumentation-openai/tests/traces/test_completions.pypackages/opentelemetry-instrumentation-replicate/opentelemetry/instrumentation/replicate/event_emitter.pypackages/opentelemetry-instrumentation-writer/opentelemetry/instrumentation/writer/__init__.pypackages/opentelemetry-instrumentation-mistralai/opentelemetry/instrumentation/mistralai/__init__.pypackages/opentelemetry-instrumentation-ollama/tests/test_generation.pypackages/opentelemetry-instrumentation-langchain/tests/conftest.pypackages/opentelemetry-instrumentation-bedrock/opentelemetry/instrumentation/bedrock/__init__.pypackages/opentelemetry-instrumentation-vertexai/opentelemetry/instrumentation/vertexai/__init__.pypackages/opentelemetry-instrumentation-anthropic/opentelemetry/instrumentation/anthropic/event_emitter.pypackages/opentelemetry-instrumentation-mistralai/tests/test_embeddings.pypackages/opentelemetry-instrumentation-openai/tests/conftest.pypackages/opentelemetry-instrumentation-bedrock/tests/traces/test_meta.pypackages/opentelemetry-instrumentation-bedrock/tests/traces/test_cohere.pypackages/opentelemetry-instrumentation-openai/tests/traces/test_embeddings.pypackages/opentelemetry-instrumentation-bedrock/tests/traces/test_anthropic.pypackages/opentelemetry-instrumentation-writer/tests/test_completions.pypackages/opentelemetry-instrumentation-openai/tests/traces/test_chat.pypackages/opentelemetry-instrumentation-sagemaker/opentelemetry/instrumentation/sagemaker/__init__.pypackages/opentelemetry-instrumentation-watsonx/opentelemetry/instrumentation/watsonx/__init__.pypackages/opentelemetry-instrumentation-sagemaker/opentelemetry/instrumentation/sagemaker/event_emitter.pypackages/opentelemetry-instrumentation-groq/opentelemetry/instrumentation/groq/event_emitter.pypackages/opentelemetry-instrumentation-cohere/opentelemetry/instrumentation/cohere/event_emitter.pypackages/opentelemetry-instrumentation-together/tests/test_completion.pypackages/opentelemetry-instrumentation-google-generativeai/opentelemetry/instrumentation/google_generativeai/event_emitter.pypackages/opentelemetry-instrumentation-ollama/opentelemetry/instrumentation/ollama/__init__.pypackages/opentelemetry-instrumentation-ollama/tests/test_chat.pypackages/opentelemetry-instrumentation-transformers/opentelemetry/instrumentation/transformers/__init__.pypackages/opentelemetry-instrumentation-writer/opentelemetry/instrumentation/writer/event_emitter.pypackages/opentelemetry-instrumentation-watsonx/tests/traces/test_generate.pypackages/opentelemetry-instrumentation-google-generativeai/opentelemetry/instrumentation/google_generativeai/__init__.pypackages/opentelemetry-instrumentation-replicate/opentelemetry/instrumentation/replicate/__init__.py
🧬 Code graph analysis (14)
packages/opentelemetry-instrumentation-watsonx/opentelemetry/instrumentation/watsonx/event_emitter.py (1)
packages/opentelemetry-instrumentation-anthropic/opentelemetry/instrumentation/anthropic/event_emitter.py (2)
_emit_message_event(187-216)_emit_choice_event(219-241)
packages/opentelemetry-instrumentation-openai/opentelemetry/instrumentation/openai/v1/__init__.py (1)
packages/opentelemetry-instrumentation-openai/opentelemetry/instrumentation/openai/shared/config.py (1)
Config(6-15)
packages/opentelemetry-instrumentation-bedrock/opentelemetry/instrumentation/bedrock/event_emitter.py (3)
packages/opentelemetry-instrumentation-google-generativeai/opentelemetry/instrumentation/google_generativeai/event_emitter.py (1)
_emit_choice_event(127-150)packages/opentelemetry-instrumentation-writer/opentelemetry/instrumentation/writer/event_emitter.py (1)
_emit_choice_event(130-150)packages/opentelemetry-instrumentation-bedrock/opentelemetry/instrumentation/bedrock/event_models.py (2)
MessageEvent(26-31)ChoiceEvent(35-41)
packages/opentelemetry-instrumentation-alephalpha/opentelemetry/instrumentation/alephalpha/__init__.py (1)
packages/opentelemetry-instrumentation-alephalpha/opentelemetry/instrumentation/alephalpha/event_models.py (1)
PromptEvent(26-31)
packages/opentelemetry-instrumentation-together/opentelemetry/instrumentation/together/event_emitter.py (2)
packages/opentelemetry-instrumentation-together/opentelemetry/instrumentation/together/event_models.py (2)
PromptEvent(26-31)CompletionEvent(35-41)packages/opentelemetry-instrumentation-anthropic/opentelemetry/instrumentation/anthropic/event_emitter.py (2)
_emit_message_event(187-216)_emit_choice_event(219-241)
packages/opentelemetry-instrumentation-replicate/opentelemetry/instrumentation/replicate/event_emitter.py (2)
packages/opentelemetry-instrumentation-replicate/opentelemetry/instrumentation/replicate/event_models.py (2)
MessageEvent(26-31)ChoiceEvent(35-41)packages/opentelemetry-instrumentation-anthropic/opentelemetry/instrumentation/anthropic/event_emitter.py (2)
_emit_message_event(187-216)_emit_choice_event(219-241)
packages/opentelemetry-instrumentation-langchain/tests/conftest.py (3)
packages/opentelemetry-instrumentation-llamaindex/tests/conftest.py (4)
fixture_logger_provider(46-49)instrument_with_content(73-86)instrument_legacy(53-69)instrument_with_no_content(90-103)packages/opentelemetry-instrumentation-openai/tests/conftest.py (4)
fixture_logger_provider(110-114)instrument_with_content(153-169)instrument_legacy(134-149)instrument_with_no_content(173-189)packages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/config.py (1)
Config(6-9)
packages/opentelemetry-instrumentation-anthropic/opentelemetry/instrumentation/anthropic/event_emitter.py (2)
packages/opentelemetry-instrumentation-cohere/opentelemetry/instrumentation/cohere/event_emitter.py (2)
_emit_message_event(90-119)_emit_choice_event(122-144)packages/opentelemetry-instrumentation-anthropic/opentelemetry/instrumentation/anthropic/event_models.py (2)
MessageEvent(26-31)ChoiceEvent(35-41)
packages/opentelemetry-instrumentation-openai/tests/conftest.py (2)
packages/opentelemetry-instrumentation-langchain/tests/conftest.py (1)
fixture_logger_provider(51-54)packages/opentelemetry-instrumentation-openai/opentelemetry/instrumentation/openai/shared/config.py (1)
Config(6-15)
packages/opentelemetry-instrumentation-sagemaker/opentelemetry/instrumentation/sagemaker/event_emitter.py (1)
packages/opentelemetry-instrumentation-anthropic/opentelemetry/instrumentation/anthropic/event_emitter.py (2)
_emit_message_event(187-216)_emit_choice_event(219-241)
packages/opentelemetry-instrumentation-groq/opentelemetry/instrumentation/groq/event_emitter.py (2)
packages/opentelemetry-instrumentation-anthropic/opentelemetry/instrumentation/anthropic/event_emitter.py (2)
_emit_message_event(187-216)_emit_choice_event(219-241)packages/opentelemetry-instrumentation-writer/opentelemetry/instrumentation/writer/event_emitter.py (2)
_emit_message_event(104-127)_emit_choice_event(130-150)
packages/opentelemetry-instrumentation-cohere/opentelemetry/instrumentation/cohere/event_emitter.py (2)
packages/opentelemetry-instrumentation-cohere/opentelemetry/instrumentation/cohere/event_models.py (2)
MessageEvent(26-31)ChoiceEvent(35-41)packages/opentelemetry-instrumentation-anthropic/opentelemetry/instrumentation/anthropic/event_emitter.py (2)
_emit_message_event(187-216)_emit_choice_event(219-241)
packages/opentelemetry-instrumentation-google-generativeai/opentelemetry/instrumentation/google_generativeai/event_emitter.py (2)
packages/opentelemetry-instrumentation-bedrock/opentelemetry/instrumentation/bedrock/event_emitter.py (2)
_emit_message_event(246-277)_emit_choice_event(280-302)packages/opentelemetry-instrumentation-google-generativeai/opentelemetry/instrumentation/google_generativeai/event_models.py (2)
MessageEvent(26-31)ChoiceEvent(35-41)
packages/opentelemetry-instrumentation-writer/opentelemetry/instrumentation/writer/event_emitter.py (1)
packages/opentelemetry-instrumentation-anthropic/opentelemetry/instrumentation/anthropic/event_emitter.py (2)
_emit_message_event(187-216)_emit_choice_event(219-241)
🪛 Ruff (0.14.5)
packages/opentelemetry-instrumentation-openai/tests/conftest.py
154-154: Unused function argument: reader
(ARG001)
154-154: Unused function argument: tracer_provider
(ARG001)
154-154: Unused function argument: meter_provider
(ARG001)
174-174: Unused function argument: reader
(ARG001)
174-174: Unused function argument: tracer_provider
(ARG001)
174-174: Unused function argument: meter_provider
(ARG001)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Test Packages (3.10)
- GitHub Check: Test Packages (3.12)
- GitHub Check: Lint
- GitHub Check: Test Packages (3.11)
- GitHub Check: Build Packages (3.11)
...ation-google-generativeai/opentelemetry/instrumentation/google_generativeai/event_emitter.py
Outdated
Show resolved
Hide resolved
...try-instrumentation-transformers/opentelemetry/instrumentation/transformers/event_emitter.py
Outdated
Show resolved
Hide resolved
galkleinman
changed the title
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed e75a73e in 3 minutes and 50 seconds. Click for details.
- Reviewed
1206lines of code in84files - Skipped
0files when reviewing. - Skipped posting
4draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. packages/opentelemetry-instrumentation-writer/tests/test_completions.py:1
- Draft comment:
Overall, the test cases for writer completions (both legacy and with events) are comprehensive. The span attribute assertions and log‐record validations appear consistent with our expected behavior (e.g. LLM_SYSTEM == "writer", proper prompt and completion content, and correct handling of stop sequences). - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is purely informative, praising the comprehensiveness of the test cases and the consistency of assertions and validations. It does not provide any actionable feedback or suggestions for improvement.
2. packages/opentelemetry-instrumentation-writer/tests/test_completions.py:45
- Draft comment:
Minor note: the stop sequence is asserted as a tuple (".",) in the legacy test. Be sure that this format matches our API’s convention and is consistent across test files. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
3. packages/opentelemetry-instrumentation-writer/tests/test_completions.py:90
- Draft comment:
The tests using events (with content and with no content) clearly verify that the correct user and choice events are emitted. Consider factoring out common test logic if these patterns repeat across files. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
4. packages/opentelemetry-instrumentation-writer/tests/test_completions.py:97
- Draft comment:
Ensure that the helper function 'assert_message_in_logs' is either defined or imported consistently in all writer tests – it appears in other test files but isn’t shown in this diff. Maintaining uniformity will help with future maintenance. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
Workflow ID: wflow_4URRmewYQVcYCOeD
You can customize
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/opentelemetry-instrumentation-google-generativeai/opentelemetry/instrumentation/google_generativeai/event_emitter.py (1)
144-150: Remove extraneous blank line in LogRecord constructor.Line 148 contains an unnecessary blank line inside the
LogRecordconstructor call, which is inconsistent with the formatting at lines 119-123.Apply this diff to fix the formatting:
log_record = LogRecord( body=body, attributes=EVENT_ATTRIBUTES, event_name="gen_ai.choice" - ) event_logger.emit(log_record)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
packages/opentelemetry-instrumentation-google-generativeai/opentelemetry/instrumentation/google_generativeai/event_emitter.py(7 hunks)packages/opentelemetry-instrumentation-langchain/tests/conftest.py(3 hunks)packages/opentelemetry-instrumentation-transformers/opentelemetry/instrumentation/transformers/event_emitter.py(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/opentelemetry-instrumentation-transformers/opentelemetry/instrumentation/transformers/event_emitter.py
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Store API keys only in environment variables/secure vaults; never hardcode secrets in code
Use Flake8 for code linting and adhere to its rules
Files:
packages/opentelemetry-instrumentation-google-generativeai/opentelemetry/instrumentation/google_generativeai/event_emitter.pypackages/opentelemetry-instrumentation-langchain/tests/conftest.py
🧬 Code graph analysis (2)
packages/opentelemetry-instrumentation-google-generativeai/opentelemetry/instrumentation/google_generativeai/event_emitter.py (1)
packages/opentelemetry-instrumentation-bedrock/opentelemetry/instrumentation/bedrock/event_emitter.py (3)
emit_message_events(33-61)_emit_message_event(246-277)_emit_choice_event(280-302)
packages/opentelemetry-instrumentation-langchain/tests/conftest.py (2)
packages/opentelemetry-instrumentation-llamaindex/tests/conftest.py (4)
fixture_logger_provider(46-49)instrument_with_content(73-86)instrument_legacy(53-69)instrument_with_no_content(90-103)packages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/config.py (1)
Config(6-9)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Test Packages (3.10)
- GitHub Check: Test Packages (3.12)
- GitHub Check: Test Packages (3.11)
- GitHub Check: Build Packages (3.11)
- GitHub Check: Lint
- GitHub Check: Analyze (python)
- GitHub Check: Analyze (actions)
🔇 Additional comments (5)
packages/opentelemetry-instrumentation-langchain/tests/conftest.py (3)
49-53: LGTM! Fixture correctly migrated to LoggerProvider.The fixture properly migrates from
EventLoggerProvidertoLoggerProviderand is correctly configured with the log exporter and processor, aligning with the OpenTelemetry 1.38.0 LogRecords API migration.
95-101: Excellent fix! Previous critical issue resolved.The fixture now correctly calls
logger_provider.get_logger(__name__, __version__)as a method on theLoggerProviderinstance, which properly addresses the critical issue flagged in the previous review. This matches the OpenTelemetry SDK 1.38.0 API and avoids theTypeErrorthat would occur with the invalidlogger_provider=parameter.
112-118: LGTM! Correctly updated to use logger_provider.The fixture properly uses
logger_provider.get_logger(__name__, __version__), consistent with theinstrument_with_contentfixture and correctly addressing the previous API usage issue.packages/opentelemetry-instrumentation-google-generativeai/opentelemetry/instrumentation/google_generativeai/event_emitter.py (2)
137-142: Verify the need to remove role when prompts are disabled.Line 139 removes the
"role"field from the message whenshould_send_prompts()returnsFalse. This differs from the Bedrock implementation, which only removes"content"and"arguments"but preserves"role".Please confirm whether removing the role is intentional for privacy/compliance or if it should be preserved like in the Bedrock implementation. The role field typically doesn't contain sensitive information and may be useful for debugging even when content is omitted.
35-35: Update type hints toOptional[Logger]and add None guards in emit functions.The functions
emit_message_events,emit_choice_events, andemit_eventdeclareevent_logger: Logger(non-optional), yet the call sites in__init__.pycheckand event_loggerbefore calling them, indicating None is possible. Without internal None guards, passing None will causeAttributeErrorwhen_emit_message_eventor_emit_choice_eventtry to call methods on it. Update toOptional[Logger]and addif event_logger is None: returnat the start ofemit_event(lines 35, 58, 74-75).
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed 81c62fe in 4 minutes and 13 seconds. Click for details.
- Reviewed
96lines of code in3files - Skipped
0files when reviewing. - Skipped posting
3draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. packages/opentelemetry-instrumentation-google-generativeai/opentelemetry/instrumentation/google_generativeai/event_emitter.py:35
- Draft comment:
Good update: type hints for 'event_logger' have been changed from Union[Logger] to Logger. The migration to using LogRecord with an event_name field is consistently applied across emit_message_events, emit_choice_events, and related helper functions. Ensure that all callers supply a valid Logger instance. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
2. packages/opentelemetry-instrumentation-langchain/tests/conftest.py:98
- Draft comment:
Replaced deprecated get_logger with logger_provider.get_logger in the 'instrument_with_content' and 'instrument_with_no_content' fixtures. This aligns with the new OpenTelemetry logging API and ensures compatibility with v1.37.0+. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
3. packages/opentelemetry-instrumentation-transformers/opentelemetry/instrumentation/transformers/event_emitter.py:4
- Draft comment:
The import for LogRecord now uses opentelemetry.sdk._logs._internal. While this works with the current SDK version, relying on an internal module may risk breaking changes in the future. Consider using a public API if one is available. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
Workflow ID: wflow_IemNWKIsktrYactC
You can customize
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed 6bec518 in 2 minutes and 43 seconds. Click for details.
- Reviewed
34lines of code in1files - Skipped
0files when reviewing. - Skipped posting
2draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. packages/opentelemetry-instrumentation-llamaindex/tests/conftest.py:76
- Draft comment:
Migration to logger_provider.get_logger is correctly applied in the 'instrument_with_content' fixture. Ensure that passing name and version is sufficient for logger identification in the new LogRecord API. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
2. packages/opentelemetry-instrumentation-llamaindex/tests/conftest.py:93
- Draft comment:
Similarly, the 'instrument_with_no_content' fixture now correctly uses logger_provider.get_logger. Confirm that name and version correctly configure the logger for the new LogRecord API. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
Workflow ID: wflow_9jVmuDuCgKQ4Cf7c
You can customize
Migrate all packages from the deprecated OpenTelemetry Events API to the
LogRecords API to support OpenTelemetry SDK 1.37.0+. The Events API was
deprecated in favor of LogRecords with event_name field.
Changes:
Bug fixes:
Fixes #3451
Important
Migrate from OpenTelemetry Events API to LogRecords API for SDK 1.37.0+ compatibility, updating dependencies and tests.
Event/EventLoggerwithLogRecord/Loggeracross packages.LogRecordwithevent_namefield.opentelemetry-instrumentationpackages to 0.59b0.log_record.event_nameinstead of attributes.pytest-asyncioconfiguration forcoherepackage.langchainandllamaindex(event_logger reference).alephalphaclient initialization and version constraints.bedrocktoken counts from HTTP headers for Anthropic V2 models.This description was created by
for 6bec518. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.