updates to trace metadata

[dinmukhamedm]

Important

Enhances trace metadata handling in Laminar SDK by introducing new methods for setting tags and metadata, deprecating older methods, and updating tests accordingly.

• Behavior:
  • Introduces set_trace_session_id(), set_trace_user_id(), and set_trace_metadata() in laminar.py for setting session, user IDs, and metadata on traces.
  • Deprecates set_session() and set_metadata() in laminar.py.
  • Adds tags parameter to observe() in decorators.py for associating tags with traces.
  • Updates entity_method() and aentity_method() in decorators.py to handle association_properties directly on spans.
• Utilities:
  • Adds is_otel_attribute_value_type() in utils.py to validate OpenTelemetry attribute types.
• Tests:
  • Adds tests for new tag and metadata functionalities in test_observe.py and test_tracing.py.
  • Removes tests for deprecated with_labels() method in test_tracing.py.

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

[ellipsis-dev]

Important

Looks good to me! 👍

Reviewed everything up to f6e38ac in 1 minute and 55 seconds. Click for details.

• Reviewed 738 lines of code in 10 files
• Skipped 0 files when reviewing.
• Skipped posting 8 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:9

• Draft comment:
  Version bump to 0.6.8 is correctly applied. Ensure the changelog (or release notes) is updated accordingly.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 50% None

2. src/lmnr/opentelemetry_lib/decorators/__init__.py:62

• Draft comment:
  The logic for setting association properties is duplicated between sync and async wrappers. Consider extracting a helper to adhere to DRY principles.
• Reason this comment was not posted:
  Confidence changes required: 33% <= threshold 50% None

3. src/lmnr/sdk/decorators.py:87

• Draft comment:
  Tag validation in the observe decorator is implemented well. Consider normalizing tag type if a consistent representation (e.g. list vs tuple) is required downstream.
• Reason this comment was not posted:
  Confidence changes required: 33% <= threshold 50% None

4. src/lmnr/sdk/laminar.py:302

• Draft comment:
  In start_as_current_span, tags are processed and passed via tag_props. Tests expect the tags attribute as a tuple. Confirm that the OTEL instrumentation normalizes the type or convert them explicitly.
• Reason this comment was not posted:
  Confidence changes required: 33% <= threshold 50% None

5. src/lmnr/sdk/laminar.py:785

• Draft comment:
  The set_trace_metadata method correctly checks the type of metadata values using is_otel_attribute_value_type. This is a good approach to handle non‐primitive types.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 50% None

6. src/lmnr/sdk/utils.py:118

• Draft comment:
  In is_otel_attribute_value_type, primitive types are checked before sequences. This order prevents strings (which are sequences) from entering the sequence branch, but consider adding a clarifying comment.
• Reason this comment was not posted:
  Confidence changes required: 33% <= threshold 50% None

7. tests/test_observe.py:266

• Draft comment:
  The test for observe tags expects the tags attribute as a tuple. Confirm that the OTEL instrumentation or your implementation converts the provided list appropriately.
• Reason this comment was not posted:
  Confidence changes required: 33% <= threshold 50% None

8. tests/test_tracing.py:228

• Draft comment:
  Typo: The function name "test_session_id_doesnt_leak" is missing an apostrophe. Consider renaming it to "test_session_id_doesn't_leak" 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 = 0% vs. threshold = 50% 1. Python style guides typically recommend using underscores in function names, not apostrophes. 2. The current name is perfectly readable and follows conventions. 3. Adding an apostrophe would actually make the name less standard. 4. This is a test function name, so readability is adequate as is. Could there be a style guide specific to this project that prefers apostrophes? Could the readability benefit outweigh the convention? No, Python's PEP 8 style guide explicitly recommends using underscores for function names, and there's no evidence of a project-specific style guide requiring apostrophes. The current name is clear and conventional. The comment should be deleted as it suggests a change that would make the code less conventional without providing any real benefit.

Workflow ID: wflow_uhNm5bb3p4zFVxpL

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

[greptile-apps]

PR Summary

Major update to trace metadata handling across the SDK, focusing on improved span attribute management and deprecation of global state.

• Deprecated global state methods (set_session, with_labels) in favor of span-specific methods (set_trace_session_id, set_span_tags) in laminar.py
• Added new is_otel_attribute_value_type validation function in utils.py to ensure proper attribute value types
• Simplified association properties by setting them directly on spans rather than using context properties system in decorators/__init__.py
• Added tags parameter to @observe decorator with validation for string lists
• Updated version to 0.6.8 to reflect these changes

_{10 file(s) reviewed, 8 comment(s)
Edit PR Review Bot Settings | Greptile}

[greptile-apps]

logic: Missing bytes type in primitive check. OpenTelemetry supports bytes as a primitive attribute type.

Suggested change

	return isinstance(value, (int, float, str, bool))
	return isinstance(value, (int, float, str, bool, bytes))

[dinmukhamedm]

that's not True at least according to the type hint in their code. https://github.com/open-telemetry/opentelemetry-python/blob/main/opentelemetry-api/src/opentelemetry/util/types.py#L31

[ellipsis-dev]

Important

Looks good to me! 👍

Reviewed 23fc8c4 in 39 seconds. Click for details.

• Reviewed 39 lines of code in 2 files
• Skipped 0 files when reviewing.
• Skipped posting 3 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. tests/test_tracing.py:5

• Draft comment:
  Removed unused 'observe' import. Good cleanup.
• Reason this comment was not posted:
  Comment did not seem useful. Confidence is useful = 0% <= threshold 50% This comment is purely informative, stating that an unused import was removed. It doesn't provide any actionable feedback or suggestions for improvement.

2. tests/test_utils.py:13

• Draft comment:
  Added test for 'None' input ensures None is rejected as a valid attribute type.
• Reason this comment was not posted:
  Comment did not seem useful. Confidence is useful = 0% <= threshold 50% This comment is purely informative, stating that a test was added for None input. It doesn't provide any suggestions or ask for confirmation on any specific aspect of the code.

3. tests/test_utils.py:23

• Draft comment:
  New nested sequences tests verify that nested lists and tuples are correctly rejected.
• Reason this comment was not posted:
  Comment did not seem useful. Confidence is useful = 0% <= threshold 50% This comment is purely informative, stating that new tests verify certain behavior. It doesn't provide any actionable feedback or suggestions for improvement.

Workflow ID: wflow_qrcAz21ip8hP5Iz8

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

[ellipsis-dev]

Important

Looks good to me! 👍

Reviewed a510e5b in 45 seconds. Click for details.

• Reviewed 65 lines of code in 2 files
• Skipped 0 files when reviewing.
• Skipped posting 3 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. src/lmnr/opentelemetry_lib/tracing/context_properties.py:13

• Draft comment:
  Updated TODO comments now mention moving logic to Laminar.set_tracing_level. Ensure the deprecation and migration strategy is coordinated across all affected functions.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 50% None

2. src/lmnr/sdk/laminar.py:736

• Draft comment:
  Replaced update_association_properties with a direct span attribute update in set_session. This explicit check for an active span is clear; ensure that the fallback behavior is acceptable when no span is active.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 50% None

3. src/lmnr/sdk/laminar.py:788

• Draft comment:
  In set_metadata, direct iteration over props to call span.set_attribute replaces update_association_properties. This approach is straightforward; consider refactoring shared logic if similar patterns appear in multiple places.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 50% None

Workflow ID: wflow_0BbvJRslFyewVBXq

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