opentelemetry-php
opentelemetry-php copied to clipboard
Forcing service resource detector prevents from using more static service instance id
Describe your environment We have:
- NewRelic
- otel-collector-contrib
- monolith php app with manual instrumentation
As it was mentioned before (here and here) - paraphrasing - if we don't like usage of uuid as service id we can write our own resource detector. Change that was introduced in PR#1573 prevents us from doing so.
Why this is important?
We use NewRelic and they require some of the fields to have low cardinality and one of hose fields is service.instance.id.
Please either move those two lines above foreach from line 41 - so those are always added BUT there is a way to override those fields (less desirable because it will create objects just to overwrite them) - or restore previous behavior where we could just not list them in OTEL_PHP_DETECTORS env variable and replace with custom implementation (less resource usage).
This is breaking our instrumentation and we had to rollback to previous versions.
Steps to reproduce Send any span or metric
What is the expected behavior?
Be able to override value service.instance.id using resource detector.
What is the actual behavior? Because service is added at the end and detectors are processed from top to bottom there is no way to override value of that field.
Additional context In my opinion otel spec is incorrect in description of this field and it is targeting only long running applications like java or .net that start once and handle thousands of requests without restart. In php world where everything lives in the request and it recreated this value should be stable.
I know you're asking to do this in a custom resource detector; but wanted to make sure you were aware you can still set a custom service instance ID manually. E.g.,
$resourceInfo = ResourceInfoFactory::defaultResource()->merge(
ResourceInfo::create(
Attributes::create([
ResourceAttributes::DEPLOYMENT_ENVIRONMENT_NAME => $deploymentEnv,
ResourceAttributes::SERVICE_INSTANCE_ID => Uuid::uuid4()->toString(),
ResourceAttributes::SERVICE_NAME => 'MyService',
ResourceAttributes::SERVICE_NAMESPACE => 'MyOrgNamespace'
]),
ResourceAttributes::SCHEMA_URL
)
);
and then pass it as the third parameter to new TracerProvider() or whatever. defaultResource() above are the automatic detectors you are referring to, so anything merged into that will override them. You could also call your custom resource detector there and merge that, or leave out the default detectors altogether and start with emptyResource().
@GrzegorzDrozd I've submitted a PR to try to address this. In your case, do you have a detector which gives you a more static instance id that can be applied after the service detector runs? We could look at moving service.instance.id into its own detector, or being able to switch it off... 🤔
@GrzegorzDrozd I've submitted a PR to try to address this. In your case, do you have a detector which gives you a more static instance id that can be applied after the service detector runs? We could look at moving service.instance.id into its own detector, or being able to switch it off... 🤔
Yes, I am using machine-id + sha of deployed git commit. This way it is much more stable and fixes issue with newrelic.
Discussion is still ongoing in the linked issue/PR. One workaround suggested would be to set OTEL_RESOURCE_ATTRIBUTES=service.instance.id=my-stable-value
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.
I think this is fixed by #1644