-
Notifications
You must be signed in to change notification settings - Fork 130
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
Azure resource providers #1228
base: main
Are you sure you want to change the base?
Azure resource providers #1228
Conversation
@trask here's my first attempt - can you help with testing? |
|
||
The following OpenTelemetry semantic conventions will be detected: | ||
|
||
| Resource attribute | VM | Functions | App Service | Containers | |
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.
can we add aks (azure Kubernetes service)?
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.
@zeitlinger do you think it's a good idea to add AKS?
cloud.provider=Azure
cloud.platform=azure_aks
cloud.resource_id=${armId},
cloud.region=${armRegion},
k8s.cluster.name=
k8s.namespace.name=
k8s.node.name=
k8s.pod.name=
k8s.pod.uid
k8s.container.name
k8s.deployment.name
k8s.deployment.uid
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.
yes, definitely. So far, I've started with the existing implementations in other languages.
Can you point me to a starting point to implement an AKS provider?
50151a0
to
9ea505d
Compare
@trask can you check? |
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 seems good to me, once we get the component owners dialed in. Nice to have some vendor coverage.
ecb2125
to
1d41f6f
Compare
@breedx-splk please check again |
...esources/src/main/java/io/opentelemetry/contrib/azure/resource/AzureAksResourceProvider.java
Outdated
Show resolved
Hide resolved
|
||
import io.opentelemetry.api.common.AttributeKey; | ||
|
||
// copied from opentelemetry-semconv-incubating |
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.
What's the motivation in copying instead of depending?
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's a policy that library instrumentation must not depend on incubating libraries - all other contrib modules do that too
Still looking good to me....though it would have been nice not to have a bunch of extra new additional code to review after an approval. 😅 |
sorry - the PR was created before the new semconv policy - didn't spot that before |
@breedx-splk should work now |
Fixes #1214
Testing:
Has not been tested in a real setup yet - but all implementations are based on the existing resource providers in other languages.
Documentation:
done
Outstanding items
Owner
Need component owners. I can be one of them.
Host image attributes
VM seems to support host.image related attributes
host.image.id
=host.image.name
=host.image.version
=(from https://learn.microsoft.com/en-us/azure/virtual-machines/instance-metadata-service?tabs=linux)
does that make sense?
Stop VM detector fast
Right now, the VM detector has a timeout of 1s - which means that it will block the startup process by 1s if we're not running in an Azure VM. How can we avoid that?
An idea is to look for the
/etc/resolv.conf
file on linux - and some special registry key on windows.Is that really the best option?