Update otel instrumentation #9285
Conversation
|
@rasswanth-s can you please review this. |
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
| @@ -353,9 +353,6 @@ def create_kubernetes_pool( | |||
| "CREATE_PRODUCER": "False", | |||
| "INMEMORY_WORKERS": "False", | |||
| "OTEL_SERVICE_NAME": f"{pool_name}", | |||
| "OTEL_PYTHON_LOG_CORRELATION": os.environ.get( | |||
There was a problem hiding this comment.
Q: We are removing this, as currently the log co-relation is not working?
There was a problem hiding this comment.
because it configures logging.basicConfig but not uvicorn logger through the YAML file.
| @@ -39,9 +44,6 @@ def set_default_attributes(cls, attributes: dict[str, str] | None = None) -> Non | |||
| cls.default_attributes[att] = attributes[att] | |||
|
|
|||
|
|
|||
| T = TypeVar("T", bound=Callable | type) | |||
|
|
|||
|
|
|||
| def instrument( | |||
There was a problem hiding this comment.
While Testing I found that, we do not capture blob storage , as we upload them directly.
I think it would be good to enable otel at the ingress controller.
For now, we could enable at the internal traefik proxy level.
Traefik has native support: https://traefik.io/blog/monitor-your-production-at-a-glance-with-traefik-3-0-and-opentelemetry/
when we remove traefik as internal proxy, we could enable at ingress controller when we switch to nginx.
There was a problem hiding this comment.
Enabling at ingress controller, would additionally give us full view of the traces .
There was a problem hiding this comment.
Locally, that's taken care of by signoz otel collector installed in the cluster default namespace.
On cloud, this is already taken care of by default.
There was a problem hiding this comment.
While Testing I found that, we do not capture blob storage , as we upload them directly.
isn't botocore instrumentation doing that for us?
Description
Instead of deploying it in the current cluster in a separate namespace, we can deploy to a separate cluster instead.
TODOs:
Affected Dependencies
List any dependencies that are required for this change.
How has this been tested?
Checklist