specs - profiling integration: Make host.id in registration message o… #900
+6
−6
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ptional `host.id` is a not well and uniquely defined attribute, see open-telemetry/semantic-conventions#581 for example. In particular on containerized environments profiling agents do see a different `host.id` than APM-agents, which makes it harder to correlate information. To being able to correlate profiling and APM information, `container.id` was identified to fit the use case best. As profiling as well as APM agents already collect and send out `container.id` with their respective data. For non containerized environment `host.id` still can be used and in such a use cases profiling agents and APM-agents should have the same understanding of `host.id`. For backwards compatibility reasons just make the argument for `host-id` in the registration message optional.
rockdaboot
reviewed
Dec 12, 2024
| * *samples-delay-ms*: A sane upper bound of the usual time taken in milliseconds by the profiling host agent between the collection of a stacktrace and it being written to the apm-agent via the [messaging socket](#cpu-profiler-trace-correlation-message). The APM-agent will assume that all profiling data related to a span has been written to the socket if a span ended at least the provided duration ago. Note that this value doesn't need to be a hard a guarantee, but it should be the 99% case so that profiling data isn't distorted in the expected case. | ||
| * *host-id*: The [`host.id` resource attribute](https://opentelemetry.io/docs/specs/semconv/attributes-registry/host/) used for the profiling data by this profiling host agent. If an APM-agent is already sending a `host.id` it MUST print a warning if the `host.id` is different and otherwise ignore the value received by the host agent. A mismatch will lead to certain correlation features (e.g. cost and CO2 consumption) not working. If an agent does not collect the `host.id` by itself, it MUST start sending the `host.id` after receiving it from the profiler host agent to ensure aforementioned correlation features work correctly. | ||
| * *samples-delay-ms*: A sane upper bound of the usual time taken in milliseconds by the profiling agent between the collection of a stacktrace and it being written to the apm-agent via the [messaging socket](#cpu-profiler-trace-correlation-message). The APM-agent will assume that all profiling data related to a span has been written to the socket if a span ended at least the provided duration ago. Note that this value doesn't need to be a hard a guarantee, but it should be the 99% case so that profiling data isn't distorted in the expected case. | ||
| * *host-id*: The [`host.id` resource attribute](https://opentelemetry.io/docs/specs/semconv/attributes-registry/host/) is an optional argument used to correlate profiling data by the profiling agent. If an APM-agent is already sending a `host.id` it MUST print a warning if the `host.id` is different and otherwise ignore the value received by the profiling agent. A mismatch will lead to certain correlation features (e.g. cost and CO2 consumption) not working. If an APM-agent does not collect the `host.id` by itself, it MUST start sending the `host.id` after receiving it from the profiling agent to ensure aforementioned correlation features work correctly. |
There was a problem hiding this comment.
When host.id is optional and no longer used for correlation, is logging a message still required (MUST)?
There was a problem hiding this comment.
If the porposed optional host.id is sent by the profiling agent and APM agent is already sending a host.id and both host.IDs are different, then yes APM agent MUST log a warning. Event if the argument is optional, a collision of host.id values should be detected and trigger a warning.
JonasKunz
approved these changes
Dec 12, 2024
Co-authored-by: Jonas Kunz <[email protected]>
Co-authored-by: Jonas Kunz <[email protected]>
xrmx
approved these changes
Dec 12, 2024
rockdaboot
approved these changes
Dec 12, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
…ptional
host.idis a not well and uniquely defined attribute, see open-telemetry/semantic-conventions#581 for example. In particular on containerized environments profiling agents do see a differenthost.idthan APM-agents, which makes it harder to correlate information. To being able to correlate profiling and APM information,container.idwas identified to fit the use case best. As profiling as well as APM agents already collect and send outcontainer.idwith their respective data. For non containerized environmenthost.idstill can be used and in such a use cases profiling agents and APM-agents should have the same understanding ofhost.id.For backwards compatibility reasons just make the argument for
host-idin the registration message optional.CODEOWNERS)To auto-merge the PR, add
/schedule YYYY-MM-DDto the PR description.