-
Notifications
You must be signed in to change notification settings - Fork 4
Weaviate Instrumentation #90
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
base: main
Are you sure you want to change the base?
Conversation
|
|
||
| def __init__(self, exception_logger: Optional[Any] = None) -> None: | ||
| super().__init__() | ||
| Config.exception_logger = exception_logger |
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.
Where is exception_logger used ?
| # Ignore errors when unwrapping connection methods | ||
| pass | ||
|
|
||
| def _get_server_details(self, version: int, tracer: Tracer) -> None: |
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.
should the function name be more meaningful ?
|
|
||
| class _WeaviateConnectionInjectionWrapper: | ||
| """ | ||
| A wrapper that intercepts calls to weaviate connection methods to inject tracing headers. |
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.
Is this description accurate ? do not see any tracing header injection happening in the logic below. Let's discuss if I am misunderstanding it ?
|
|
||
| class _WeaviateTraceInjectionWrapper: | ||
| """ | ||
| A wrapper that intercepts calls to weaviate to inject tracing headers. |
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.
Is this description accurate ? do not see any tracing header injection happening in the logic below. Let's discuss if I am misunderstanding it ?
| getattr(wrapped, "__name__", "unknown"), | ||
| ) | ||
| name = f"{SPAN_NAME_PREFIX}.{name}" | ||
| with self.tracer.start_as_current_span(name, kind=SpanKind.CLIENT) as span: |
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.
It is creating a span. Should this logic be in a separate file and not in the init.py ?
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.
This is interesting as Weaviate doesn't have any underlying GenAI utils, so it is up for discretion as to where to create the span. I believe this is also what openllemetry was also doing, but I could be wrong.
add support for weaviate instrumentation