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

Allow to reuse more code from existing SDK, for example resource host detector #1489

Open
GrzegorzDrozd opened this issue Jan 27, 2025 · 3 comments

Comments

@GrzegorzDrozd
Copy link

GrzegorzDrozd commented Jan 27, 2025

service.instance.id should be used to create a unique combination of service.namespace, service.name,service.instance.id. I could use host.id for this purpose as it is detected in \OpenTelemetry\SDK\Resource\Detectors\Host::getMachineId. However, that method is private and accessible only via getResource, which performs additional tasks beyond what I need. Furthermore, getResource uses static methods across different objects, and I cannot guarantee that these implementations won't change in the future, potentially affecting data collection.

I have noticed similar patterns in other places. While I understand that adding methods like getAttribute directly on a span is discouraged, limiting access to system information in such a restrictive way seems problematic.

Is your feature request related to a problem?
Yes, the lack of a standardized way to retrieve host.id for use in attributes (either directly or as part of a hash, for example).

Describe the solution you'd like

  1. Make the getMachineId method publicly accessible and static.
  2. Review the SDK to identify and address other areas where increased openness could be beneficial.

Describe alternatives you've considered
Re-implementing the logic from getMachineId independently.

@brettmc
Copy link
Collaborator

brettmc commented Jan 28, 2025

The service resource detector generates a UUID for service.instance.id which aligns with the OpenTelemetry specification

@GrzegorzDrozd
Copy link
Author

Service resource detector generates random instance ID for every request and I thought it should be stable for the lifetime of the deployed instance?

@brettmc
Copy link
Collaborator

brettmc commented Jan 29, 2025

I don't think providing public access to the internals of parts of our SDK is a good approach. If the problem is a stable instance id when PHP is running behind a web server (fcgi/apache/etc), maybe we should look at what is a sensible value to use for instance id, and create a contrib resource detector to solve this problem?

The relevant part of the spec to keep in mind:

UUIDs are typically recommended, as only an opaque value for the purposes of identifying a service instance is needed. Similar to what can be seen in the man page for the /etc/machine-id file, the underlying data, such as pod name and namespace should be treated as confidential, being the user’s choice to expose it or not via another resource attribute.

For applications running behind an application server (like unicorn), we do not recommend using one identifier for all processes participating in the application. Instead, it’s recommended each division (e.g. a worker thread in unicorn) to have its own instance.id.

Could any of these values be used?

  • parent process id (which should be the pid of apache/fcgi), via posix_getppid. not available on windows, and does require the posix extension. Value changes if a webserver restarts (which is probably desirable)
  • hostname. should work everywhere. multiple webserver instances on the same host would have the same instance id

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

No branches or pull requests

2 participants