fix: add gentrace.in_experiment to span attributes#374
Conversation
How to use the Graphite Merge QueueAdd the label merge-queue to this PR to add it to the merge queue. You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
| context_with_modified_baggage = otel_baggage.set_baggage( | ||
| ATTR_GENTRACE_IN_EXPERIMENT, "true", context=current_context | ||
| ) |
There was a problem hiding this comment.
The current implementation overwrites the baggage context rather than extending it. The second call to set_baggage should use the result from the first call as its context parameter:
context_with_modified_baggage = otel_baggage.set_baggage(
ATTR_GENTRACE_IN_EXPERIMENT, "true", context=context_with_modified_baggage
)This ensures both baggage items are preserved in the context. The same issue appears in the eval_dataset.py file as well.
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
There was a problem hiding this comment.
Oh yeah this looks bad @joshlebed - did we check if gentrace.sample still gets set?
There was a problem hiding this comment.
Another way to validate that both are sent could be to set the OTLP trace exporter path to the local collector at 4318 and then see if it shows both span attributes
| "eval_dataset", | ||
| "TestInput", | ||
| "ATTR_GENTRACE_SAMPLE_KEY", | ||
| "ATTR_GENTRACE_IN_EXPERIMENT", |
There was a problem hiding this comment.
There appears to be a duplicate export of ATTR_GENTRACE_IN_EXPERIMENT in the __init__.py file. It's being exported once on line 173 and again on line 177 (after the changes). To maintain clean and consistent exports, please remove one of these duplicate entries.
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
| context_with_modified_baggage = otel_baggage.set_baggage( | ||
| ATTR_GENTRACE_IN_EXPERIMENT, "true", context=current_context | ||
| ) |
There was a problem hiding this comment.
Oh yeah this looks bad @joshlebed - did we check if gentrace.sample still gets set?
| context_with_modified_baggage = otel_baggage.set_baggage( | ||
| ATTR_GENTRACE_IN_EXPERIMENT, "true", context=current_context | ||
| ) |
There was a problem hiding this comment.
Another way to validate that both are sent could be to set the OTLP trace exporter path to the local collector at 4318 and then see if it shows both span attributes
| context_with_modified_baggage = otel_baggage.set_baggage( | ||
| ATTR_GENTRACE_IN_EXPERIMENT, "true", context=current_context | ||
| ) |
There was a problem hiding this comment.
There's an issue with the context parameter in this baggage setting. The code is using current_context instead of context_with_modified_baggage, which would overwrite rather than build upon the previous baggage modification. To maintain both baggage values, please update to use context_with_modified_baggage as the context parameter, similar to the implementation in eval.py:
context_with_modified_baggage = otel_baggage.set_baggage(
ATTR_GENTRACE_IN_EXPERIMENT, "true", context=context_with_modified_baggage
)This ensures that both the sample key and in_experiment attributes are properly preserved in the baggage context.
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
2 participants
TL;DR
Added a new attribute
ATTR_GENTRACE_IN_EXPERIMENTto track when spans are part of an experiment.What changed?
ATTR_GENTRACE_IN_EXPERIMENTto track when spans are created within an experiment context__init__.pyHow to test?
gentrace.eval()orgentrace.eval_dataset()gentrace.in_experimentattribute set to "true"Why make this change?
This change enables better tracking of spans that are specifically created within experiment contexts. By adding a dedicated attribute for experiment participation, it becomes easier to filter and analyze spans that are part of experiments versus those that are not, improving observability and data analysis capabilities.