Skip to content
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: inject OTEL_SERVICE_NAME and OTEL_RESOURCE_ATTRIBUTES via webhook #2008

Merged
merged 6 commits into from
Dec 16, 2024

Conversation

blumamir
Copy link
Collaborator

For dotnet and OSS java which currently relies on the resource to inject the device id in odiglet, and then resolves it from kublet to the desired service name

Copy link
Contributor

@edeNFed edeNFed left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💯

@RonFed
Copy link
Collaborator

RonFed commented Dec 16, 2024

Can we remove collector/processors/odigosresourcenameprocessor after this PR?

@blumamir
Copy link
Collaborator Author

Can we remove collector/processors/odigosresourcenameprocessor after this PR?

If we remove the processor, then after the first upgrade, there will still be pods that started without the webhook and if the processor is not there, the service name will show up as UUID. I suggest to keep it around for some-time until we fill comfortable that everyone has already upgraded, and then remove the processor

@blumamir blumamir changed the title feat: inject OTEL_SERVICE_NAME via webhook feat: inject OTEL_SERVICE_NAME and OTEL_RESOURCE_ATTRIBUTES via webhook Dec 16, 2024
containerutils "github.com/odigos-io/odigos/k8sutils/pkg/container"
"github.com/odigos-io/odigos/k8sutils/pkg/workload"
"go.opentelemetry.io/otel/attribute"
semconv "go.opentelemetry.io/otel/semconv/v1.4.0"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should use a more recent semconv

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good catch.
updated t0 1.26.0 which is what we use in other places in the codebase

@blumamir blumamir merged commit 34ac66a into odigos-io:main Dec 16, 2024
30 of 31 checks passed
Copy link

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants