-
Notifications
You must be signed in to change notification settings - Fork 408
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(llmobs): add prompt and name arguments to annotation context #10711
base: main
Are you sure you want to change the base?
Conversation
BenchmarksBenchmark execution time: 2024-09-23 18:56:36 Comparing candidate commit 235f3fd in PR branch Found 4 performance improvements and 2 performance regressions! Performance is the same for 340 metrics, 46 unstable metrics. scenario:iast_aspects-aspect_iast_do_center
scenario:iast_aspects-aspect_iast_do_index
scenario:iast_aspects-aspect_iast_do_lstrip
scenario:iast_aspects-aspect_iast_do_modulo
scenario:iast_aspects-aspect_iast_do_str
scenario:iast_aspects-aspect_iast_do_upper
|
….yaml Co-authored-by: Yun Kim <35776586+Yun-Kim@users.noreply.github.com>
|
""" | ||
Sets specified attributes on all LLMObs spans created while the returned AnnotationContext is active. | ||
Do not use nested annotation contexts to override the same tags since the order in which annotations | ||
Do not use nested annotation contexts to override the same attributes since the order in which annotations | ||
are applied is non-deterministic. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is it non-deterministic?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We register the annotation function as a hook on span start. Because these hooks are stored as a set, the order in which these hooks are called is non-deterministic
@@ -230,16 +230,23 @@ def disable(cls) -> None: | |||
log.debug("%s disabled", cls.__name__) | |||
|
|||
@classmethod | |||
def annotation_context(cls, tags: Optional[Dict[str, Any]] = None) -> AnnotationContext: | |||
def annotation_context( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's make documentation for this in our public docs, probably next to Annotate a span
.
This PR adds support for
prompt
andname
arguments toannotation_context
.Because
prompt
depends on span kind being equal tollm
, we also need to do a small refactor ofannotate
. Prompts will be tagged inannotate
before span kind is checked. The span kind checked is then done at trace processing time, where we will log a warning and drop theprompt
field for any non-LLM span kinds.Checklist
Reviewer Checklist