chore(weave): allow otel style ids for calls #3946
Conversation
|
Preview this PR with FeatureBee: https://beta.wandb.ai/?betaVersion=226fc4f4e377fb969c63a6b25dff20695309c7f9 |
zbirenbaum
force-pushed
the
zach/otel-trace-server
branch
from
9957695 to
026bb33
Compare
weave/trace_server/validation.py
Outdated
| return validation_util.require_uuid(s) | ||
| try: | ||
| return validation_util.require_otel_span_id(s) | ||
| except: |
There was a problem hiding this comment.
Need more explicit except here. What are you catching?
There was a problem hiding this comment.
Logically this is similar to require_otel_span_id(s) or require_uuid(s). The exceptions raised in the require methods cause execution to halt so this just checks if it is an OTEL UUID before allowing the require_uuid to throw.
I agree its a bit messy, I guess I could modify the original require_uuid to also check for otel format IDs if that's better, or return either a string or error from the requires and throw if it's an instance of error
There was a problem hiding this comment.
If you want to be super general then you can do except Exception which will catch most things. Bare except is dangerous because it will also catch stuff like KeyboardInterrupt which you probably dont want
There was a problem hiding this comment.
I updated it to catch the specific exception. I still think there are other ways of doing this that are cleaner but not sure its high priority and would have slightly higher footprint for sure
Description
Modify regex matching on frontend to match any ID with or without dashes to support OTEL style ID strings. Change validation of call IDs and trace IDs to include OTEL style ID strings.