-
Notifications
You must be signed in to change notification settings - Fork 621
feat(resource-detector-azure): add detector for Azure Container Apps #3289
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
Co-authored-by: James Thompson <[email protected]>
Co-authored-by: James Thompson <[email protected]>
Co-authored-by: James Thompson <[email protected]>
Co-authored-by: James Thompson <[email protected]>
|
@forsen can you update the implementation to match the changes to the readme? |
| process.env = originalEnv; | ||
| }); | ||
|
|
||
| it('should detect Azure Container Apps resource attributes', () => { |
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.
As all of our other resource detectors are designed to run alongside one another, it'd be good to add a test here to ensure that multiple resource detectors can be registered with no interference.
I trust isAzureContainerApps to keep this scoped correctly, but this would be good to have to detect any possible regressions in the future.
Which problem is this PR solving?
This PR adds detector for Azure Container Apps.
Short description of the changes
Adds detector for Azure Container Apps, detecting the built-in environment variables:
https://learn.microsoft.com/en-us/azure/container-apps/environment-variables?tabs=portal#apps
Tried to use attributes from semconv where applicable, using custom ones otherwise.
I've not implemented functionality to fetch metadata from Azure metadata endpoint. I'm fairly new with Open Telemetry, so consider the attribute mapping as a suggestion only.