profiles: remove default_sample_type#679
Conversation
7a98df8 to
23bdc47
Compare
0601985 to
cb85d1c
Compare
|
@felixge might be good to have a changelog entry? |
The profiling signal is still experimental and WIP, we don't add changelog entries for its changes right now. We will start doing this once 1.0 version is out. |
|
In the last SIG meeting I promised that I would provide an example of how a Example pprof input (inspired by the Go memory profiles, but simplified): Converted to OpenTelemetry: This should round trip back to the original pprof file. However, there is a small ambiguity here when Anyway, I don't think this is a blocker for this PR. It's certainly a solvable problem. |
This resolves the ambiguity when a `ScopesProfiles` message contains more than one `profiles` entry matching the `default_sample_type`. Also add a comment to clarify which profile viewers should display by default. This was previously also ambiguous. This change creates a minor problem when it comes to ensuring that pprof profiles can round-trip through otel conversion as it may require the `ScopeProfiles.profiles` entries to be reordered in order to honor the `default_sample_type` of the original pprof message. Naively converting the resulting otel profile back to pprof would cause the order of `Profile.sample_types` to be changed. Merging this change indiciates consensus within the profiling group that this issue should be handled by adding a `pprof.profile_order` attribute (name TBD) to the `ScopeProfiles.attributes` during the initial pprof->otel conversion. This label will allow converting the resulting otel profile back to pprof without any information loss. See [open-telemetrygh-633][] for additional details. [open-telemetrygh-633]: open-telemetry#633 (comment)
cb85d1c to
a13103f
Compare
|
This should be ready for merge now 🙇 . My force-push was a rebase. |
### Changed - profiles: drop gzip requirement. [#661](#661) - profiles: avoid `optional` keyword usage. [#659](#659) - profiles: make `profile_id` optional. [#665](#665) - profiles: use single `Profile.sample_type` and clarify use of timestamps. [#649](#649) - all: add notes about the attribute values restrictions. [#683](https://github.com/open-telemetry/opentelemetry-proto/pull/683)<br>⚠️ **IMPORTANT**: These restrictions can be dropped in a future minor release. - profiles: clarify usage of the zero value as the first element of tables in `ProfilesDictionary`. [#688](#688), [#698](#698) - profiles: unsigned `time_nanos` and `duration_nanos` in `Profile`. [#692](#692) - profiles: improve attribute encoding in `ProfilesDictionary`. [#672](#672) - profiles: simplify profile stack trace representation. [#708](#708) ### Fixed - examples: fix OTLP JSON Event example body. [#666](#666) - docs: minor specification fixes around `UNAVAILABLE` and `RetryInfo`. [#669](#669) ### Removed - profiles: remove `default_sample_type`. [#679](#679) - profiles: remove `has_*` debug info fields, they are moving to attributes. [#595](#595) - profiles: remove `Location.is_folded`. [#690](#690)
This resolves the ambiguity when a
ScopesProfilesmessage containsmore than one
profilesentry matching thedefault_sample_type.Also add a comment to clarify which profile viewers should display by
default. This was previously also ambiguous.
This change creates a minor problem when it comes to ensuring that pprof
profiles can round-trip through otel conversion as it may require the
ScopeProfiles.profilesentries to be reordered in order to honor thedefault_sample_typeof the original pprof message. Naively convertingthe resulting otel profile back to pprof would cause the order of
Profile.sample_typesto be changed.Merging this change indiciates consensus within the profiling group that
this issue should be handled by adding a
pprof.profile_orderattribute(name TBD) to the
ScopeProfiles.attributesduring the initialpprof->otel conversion. This label will allow converting the resulting
otel profile back to pprof without any information loss.
See gh-633 for additional details.