search

microsoft / ApplicationInsights-Kubernetes

Enrich the telemetry data for .NET applications running inside containers that are managed by Kubernetes.
Other
135 stars 57 forks source link

Container id shall be optional #345

Closed xiaomi7732 closed 1 year ago

xiaomi7732 commented 1 year ago

Trying to address #344

This is a regression that when there are more than 1 container, and the container id can't be fetched, no telemetry enhancement.

That doesn't agree with the design intention that container id to be optional.

Fix by allowing container id to be absent but other telemetry enhancement still happens.

Here's an example of the logs after the fix:

[Warning] [2023-03-28T04:18:44.8583934Z] Can't fetch container id. Container id info will be missing. Please file an issue at https://github.com/microsoft/ApplicationInsights-Kubernetes/issues.
[Debug] [2023-03-28T04:18:44.9234011Z] {"ContainerID":null,"ContainerName":null,"PodName":"ai-k8s-f5webapi-deployment-77dc74c9dc-p75vq","PodID":"380deca3-e9c1-405f-b5b0-8e22660a8942","PodLabels":"app:ai-k8s-f5webapi,pod-template-hash:77dc74c9dc","PodNamespace":"ai-k8s-demo","ReplicaSetUid":"87f84c9c-55d2-47d5-82c1-dd6728454b25","ReplicaSetName":"ai-k8s-f5webapi-deployment-77dc74c9dc","DeploymentUid":"c1de6986-b3b1-4f98-b898-008246c0dff5","DeploymentName":"ai-k8s-f5webapi-deployment","NodeName":"docker-desktop","NodeUid":"cd59d76e-eaf2-4408-9083-aa2d76548055"}
karolz-ms commented 1 year ago

@xiaomi7732 and I had a conversation about this. We agreed it will be better to stop requiring container readiness before container info is fetched from the API server. A new PR (or substantial revision to this PR) is in the works.

xiaomi7732 commented 1 year ago

See #348