write output schema definition to the span attributes

[dinmukhamedm]

Important

Adds functionality to write output schema definitions to span attributes in Google GenAI instrumentation, with updated error handling and new tests.

• Behavior:
  • Writes response_schema and response_json_schema to span attributes in _set_request_attributes() in __init__.py.
  • Updates error handling in _wrap() and _awrap() to record exceptions and set error status.
• Utilities:
  • Adds process_schema() and SchemaJSONEncoder in schema_utils.py to process and encode schemas.
• Tests:
  • Adds tests test_google_genai_output_schema and test_google_genai_output_json_schema in test_google_genai.py.
  • Adds VCR cassettes test_google_genai_output_schema.yaml and test_google_genai_output_json_schema.yaml for schema tests.
• Dependencies:
  • Updates opentelemetry-semantic-conventions-ai to >=0.4.9 in pyproject.toml.

^{This description was created by for 3c98ead. You can customize this summary. It will automatically update as commits are pushed.}

[greptile-apps]

Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.

[ellipsis-dev]

Important

Looks good to me! 👍

Reviewed everything up to 6a2ea77 in 1 minute and 49 seconds. Click for details.

• Reviewed 505 lines of code in 6 files
• Skipped 0 files when reviewing.
• Skipped posting 10 draft 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. pyproject.toml:25

• Draft comment:
  Dependency bump for opentelemetry-semantic-conventions-ai (>=0.4.9) looks appropriate; ensure compatibility with dependent modules.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 50% None

2. src/lmnr/opentelemetry_lib/opentelemetry/instrumentation/google_genai/__init__.py:81

• Draft comment:
  Updating the env variable from TRACELOOP_TRACE_CONTENT to LAMINAR_TRACE_CONTENT improves naming consistency; verify that all related configurations are updated.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 50% None

3. src/lmnr/opentelemetry_lib/opentelemetry/instrumentation/google_genai/__init__.py:132

• Draft comment:
  When processing output schemas in _set_request_attributes, exceptions are silently caught. Consider logging any exceptions to aid in debugging schema conversion issues.
• Reason this comment was not posted:
  Confidence changes required: 50% <= threshold 50% None

4. src/lmnr/opentelemetry_lib/opentelemetry/instrumentation/google_genai/__init__.py:486

• Draft comment:
  The try/except blocks in both _wrap and _awrap properly record error attributes. Double-check that model identifier formatting (e.g. presence of 'models/' prefix) is consistent as per API specs.
• Reason this comment was not posted:
  Confidence changes required: 30% <= threshold 50% None

5. src/lmnr/opentelemetry_lib/opentelemetry/instrumentation/google_genai/schema_utils.py:11

• Draft comment:
  Instantiating BaseApiClient with a dummy API key in process_schema may be inefficient if called frequently; consider caching the client or using a lightweight transformation method.
• Reason this comment was not posted:
  Confidence changes required: 50% <= threshold 50% None

6. tests/test_google_genai.py:457

• Draft comment:
  New tests for output schema attributes are comprehensive; ensure that the schema properties and ordering match the client expectations.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 50% None

7. tests/test_google_genai.py:60

• Draft comment:
  Note the difference in model identifiers between tests (e.g., 'models/gemini-2.5-flash-preview-05-20' vs 'gemini-2.5-flash-lite-preview-06-17'). Confirm that this is intentional as per API responses.
• Reason this comment was not posted:
  Confidence changes required: 30% <= threshold 50% None

8. tests/cassettes/test_google_genai/test_google_genai_output_json_schema.yaml:7

• Draft comment:
  Typographical concern: The schema title for 'dayOfWeek' is given as "Dayofweek". Consider using a consistent casing, e.g., "DayOfWeek", to match the property name.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% This is a test cassette file that records API interactions for testing purposes. The casing inconsistency is minor and doesn't affect functionality. The file appears to be recording actual API responses, so the casing might be coming from the API itself. Making the suggested change could make the test cassette not match real responses. The inconsistent casing could indicate a real issue in the API schema definition. Test files should still maintain good practices. Since this is a recorded API interaction, we should preserve the exact response rather than modify it to match our preferred style. Changing it could mask actual API behavior. Delete the comment because this is a test cassette recording actual API responses, and we should preserve the exact format returned by the API.

9. tests/cassettes/test_google_genai/test_google_genai_output_schema.yaml:6

• Draft comment:
  Typo detected: The title value for the property 'dayOfWeek' is written as "Dayofweek". For consistency, consider changing it to "DayOfWeek".
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% This is a test cassette file that records actual API interactions. The casing of "Dayofweek" vs "DayOfWeek" likely reflects the actual API response or request that was made. Changing it would make the test cassette inaccurate. Test cassettes should record exactly what happened, not what we wish had happened. Additionally, this is just a title field used for documentation, not affecting functionality. Maybe the casing inconsistency reflects a real bug in the API schema that should be fixed in the source code? Even if there is an underlying issue, it should be fixed in the actual API code, not in the test cassette that records the interaction. The cassette should stay true to what actually happened. Delete this comment. Test cassettes should record actual interactions exactly as they occurred, and modifying the casing would make the test data inaccurate.

10. tests/test_google_genai.py:414

• Draft comment:
  Typographical suggestion: The title 'Dayofweek' seems inconsistent with the field name 'dayOfWeek'. Consider changing it to 'Day of Week' or 'DayOfWeek' for consistency.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% This is a minor style issue about casing consistency between a field name and its title. While it's a valid point about consistency, it's not a critical issue that affects functionality. The tests will pass either way. The comment is about code that was changed in this PR (the schema was added), but the suggestion is purely cosmetic. The inconsistency might be intentional - perhaps "Dayofweek" is meant to be more readable for humans while "dayOfWeek" follows programming conventions. Also, this is just test code. While it's test code, consistency is still valuable for maintainability and professionalism. The title is meant to be human-readable, so matching the camelCase field name would actually be less readable. Delete the comment. While it makes a valid point about consistency, it's too minor of a style issue to warrant a comment, especially in test code.

Workflow ID: wflow_GtV1wuELXjsa2iJM

^{You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}

[ellipsis-dev]

Caution

Changes requested ❌

Reviewed 3c98ead in 2 minutes and 2 seconds. Click for details.

• Reviewed 505 lines of code in 6 files
• Skipped 0 files when reviewing.
• Skipped posting 7 draft 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. pyproject.toml:25

• Draft comment:
  Dependency bump: version for opentelemetry-semantic-conventions-ai updated from 0.4.8 to 0.4.9. Please ensure this change is fully backwards‐compatible with the rest of the project.
• Reason this comment was not posted:
  Comment did not seem useful. Confidence is useful = 0% <= threshold 50% This comment is about a dependency version change, which is not allowed according to the rules. The comment asks the author to ensure compatibility, which is also not allowed.

2. src/lmnr/opentelemetry_lib/opentelemetry/instrumentation/google_genai/__init__.py:487

• Draft comment:
  Instead of using 'raise e' in the exception blocks of _wrap and _awrap, consider using 'raise' alone to preserve the original traceback.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% While technically correct that 'raise' preserves more traceback information than 'raise e', in this case the code is deliberately handling the exception to record information in spans before re-raising. The difference in traceback preservation is minor and unlikely to impact debugging since the original exception type and message are preserved. The current code is clear in its intent to re-raise the caught exception. The comment is technically accurate about Python best practices. In some cases, losing traceback information could make debugging more difficult. The benefit of clearer traceback information is outweighed by the code's clear intent and the fact that essential error information is still preserved and explicitly logged to spans. The comment should be deleted as the current code is sufficiently clear and the suggested change would provide minimal benefit while potentially making the code's intent less obvious.

3. src/lmnr/opentelemetry_lib/opentelemetry/instrumentation/google_genai/__init__.py:133

• Draft comment:
  Note that if both 'response_schema' and 'response_json_schema' are present in the config, only 'response_schema' will be processed. Confirm that this behavior is intentional and documented.
• Reason this comment was not posted:
  Comment did not seem useful. Confidence is useful = 40% <= threshold 50% The comment is asking the PR author to confirm the behavior, which violates the rule against asking for confirmation. However, it also points out a specific behavior that might need attention, which could be useful. The comment could be rephrased to suggest checking the documentation or ensuring the behavior is correct without asking for confirmation.

4. src/lmnr/opentelemetry_lib/opentelemetry/instrumentation/google_genai/schema_utils.py:9

• Draft comment:
  The process_schema function creates a new dummy BaseApiClient instance on each call. Consider caching the processed schema if the same schema is frequently used to improve performance.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% While creating a new client each time could be inefficient, we don't know: 1) If this function is called frequently enough for caching to matter 2) If schemas are commonly reused 3) If BaseApiClient creation is actually expensive 4) If caching would introduce thread-safety issues. The comment is speculative without evidence of a performance problem. The comment identifies a potential performance optimization. Creating objects repeatedly can be inefficient. Without evidence that this is a real performance bottleneck or that schemas are commonly reused, this is premature optimization. The current implementation is simpler and more obviously correct. Delete the comment. While it suggests a possible optimization, it's speculative without evidence of actual performance impact or schema reuse patterns.

5. tests/cassettes/test_google_genai/test_google_genai_output_json_schema.yaml:7

• Draft comment:
  Typo detected: The title for the 'dayOfWeek' property is set as 'Dayofweek'. Consider using a consistent capitalization (e.g., 'DayOfWeek' or 'Day Of Week') for clarity.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% This is a test cassette file that records API interactions for testing purposes. The capitalization inconsistency is minor and doesn't affect functionality. The file appears to be recording actual API responses, so changing the capitalization might make the test not match real responses. Test cassettes should generally reflect actual API behavior rather than be manually edited for style. The inconsistent capitalization could make the API response harder to read and understand. It might indicate an actual bug in the API schema definition. While readability is important, this is a test cassette that should accurately record API behavior. If there's a capitalization issue, it should be fixed in the API itself, not in the test recording. Delete the comment as it suggests modifying a test cassette that should accurately reflect API responses, even if those responses have minor style issues.

6. tests/cassettes/test_google_genai/test_google_genai_output_schema.yaml:6

• Draft comment:
  Typo: The title for the 'dayOfWeek' property is written as "Dayofweek". Consider using "DayOfWeek" or "Day of Week" for consistency.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% This is a test cassette file that records actual API interactions for testing purposes. The casing inconsistency might be reflecting the actual API behavior or response format. Changing it could make the test data inaccurate. Test cassettes should generally reflect the real interaction rather than be "cleaned up". Additionally, this is a very minor stylistic issue that doesn't affect functionality. The inconsistent casing could make the code less readable and maintainable if this schema is used as documentation or reference. The primary purpose of test cassettes is to accurately record API interactions, not to enforce style conventions. Modifying the actual response for style would defeat the purpose. Delete this comment as it suggests modifying test data that should accurately reflect the real API interaction.

7. tests/test_google_genai.py:414

• Draft comment:
  Typo: In the expected schema for 'dayOfWeek', the title is written as "Dayofweek". Consider changing it to "DayOfWeek" (or "Day of Week") for consistency with the field name.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% This is a test file and the schema is just used for testing API responses. The casing difference is minor and doesn't affect functionality. The schema works fine either way since it's just a display title. The actual field name dayOfWeek is what matters for the JSON structure. The inconsistent casing could be confusing to other developers reading the code. Having consistent casing throughout makes the code more professional. While consistency is nice, this is a minor cosmetic issue in test code that doesn't impact functionality. The comment adds noise without providing significant value. Delete the comment. The casing inconsistency is too minor of an issue to warrant a PR comment, especially in test code.

Workflow ID: wflow_lQA50lThtRhNyJMX

^{You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}

[ellipsis-dev]

In the schema processing block (_set_request_attributes), exceptions are silently caught. Consider logging the error to aid in debugging schema conversion issues.