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

add service.instance.id provider #9062

Conversation

zeitlinger
Copy link
Member

service.instance.id is an important attribute to support Prometheus: https://opentelemetry.io/docs/specs/otel/compatibility/prometheus_and_openmetrics/#resource-attributes

@zeitlinger zeitlinger requested a review from a team July 27, 2023 10:19
@github-actions github-actions bot requested a review from theletterf July 27, 2023 10:19
Copy link
Member

@mateuszrzeszutek mateuszrzeszutek left a comment

Choose a reason for hiding this comment

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

There is an open spec issue that suggests adding service.instance.id generation to the SDK itself: open-telemetry/opentelemetry-specification#3136
I'm not sure whether we should add this here; maybe a prototype in the SDK and pushing the spec issue would be a better course of action?
cc @jack-berg

Comment on lines +36 to +40
private static String k8sServiceInstanceId(Map<String, String> resource) {
String podName = resource.get(ResourceAttributes.K8S_POD_NAME.getKey());
String containerName = resource.get(ResourceAttributes.K8S_CONTAINER_NAME.getKey());
return podName != null && containerName != null ? podName + "/" + containerName : null;
}
Copy link
Member

Choose a reason for hiding this comment

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

The spec doesn't really say anything about using k8s attributes to construct the serviceId (also k8s attributes are usually added by the collector processors, these might simply not be accessible here)

Comment on lines +23 to +29

@Override
public boolean shouldApply(ConfigProperties config, Resource existing) {
return !config
.getMap(ServiceInstanceIdResource.RESOURCE_ATTRIBUTES)
.containsKey(ResourceAttributes.SERVICE_INSTANCE_ID.getKey());
}
Copy link
Member

Choose a reason for hiding this comment

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

This shouldn't really be needed, the EnvironmentResource always comes last and it can override that anyway.

@zeitlinger
Copy link
Member Author

Superceded by open-telemetry/opentelemetry-java#6226

@zeitlinger zeitlinger closed this Feb 13, 2024
@zeitlinger zeitlinger deleted the service_instance_id_provider branch February 13, 2024 08:15
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.

2 participants