WIP: add profiling signal #1
Draft
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
…ields (open-telemetry#442) Resolves open-telemetry/opentelemetry-specification#3040 This is not a breaking change: - For Span it now defines more precisely the receiver behavior that was previously defined vaguely (e.g. it was unclear what "empty" means for bytes field). - For LogRecord it now defines the receiver behavior that was previously unspecified. This ensures that the wording are consistent with what we have for the Span.
…-telemetry#454) * Update breaking-change to check against last published version * Fetch tags
Co-authored-by: Reiley Yang <reyang@microsoft.com> Co-authored-by: Armin Ruech <armin.ruech@dynatrace.com>
This PR explicitly lists the additive changes allowed and adds a requirement that such additive changes must be accompanied by interoperability explanation when necessary. This is a subset of open-telemetry#432 that contains other guarantees that we did not yet agree to. I believe this particular subset is necessary regardless of what we decide about open-telemetry#432.
Signed-off-by: Bogdan Drutu <bogdandrutu@gmail.com>
* Add OTLP to the specification We previously had OTLP definition scattered in multiple OTEPs. This amalgamates the OTEPs and brings OTLP to the specification repo. * Address PR comments * Address PR comments
* Allow specifying a single configuration The following change describes that the OTLP exporter must support configuration for all signals via a single set of configuration options. There is also an example for configuring different signals with different endpoints via environment variables. * adding a third example * moving otlp exporter into protocol directory * add link to exporter from readme * apply review feedback
…g (#911) Resolves: open-telemetry/opentelemetry-specification#786 See discussion and motivation for the change in the issue linked above. Co-authored-by: Sergey Kanzhelev <S.Kanzhelev@live.com>
MUST is the correct requirement here. There is no situation when a different content type is a better choice.
* Change default OTLP port number Contributes to open-telemetry/opentelemetry-specification#1148 Note that a separate port is used for OTLP/HTTP for now. There is currently work in progress to confirm that we can use the same port. Once we have the confirmation I will update the spec again to use one port. * Address PR comments
* Remove support to allow_different_nesting, from markdownlint Signed-off-by: Bogdan Drutu <bogdandrutu@gmail.com> * Update specification/metrics/semantic_conventions/README.md Co-authored-by: Armin Ruech <armin.ruech@dynatrace.com> * Run markdown-toc Signed-off-by: Bogdan Drutu <bogdandrutu@gmail.com> Co-authored-by: Armin Ruech <armin.ruech@dynatrace.com>
## Changes It seems wired that the closing tag of `<details>` is missing. As a result, most of the content has been folded. Added a closing `</details>` tags to only fold the table of contents. Before the change:  After the change: 
* Versioning and support based on OTEP 143
Signed-off-by: Bogdan Drutu <bogdandrutu@gmail.com>
…(#1637) Resolves open-telemetry#268 This behavior is already the documented behavior for JSON Protobuf, see https://developers.google.com/protocol-buffers/docs/proto3#json: >JSON value will be a decimal string. Either numbers or strings are accepted.
* Mark sections of datamodel stable. * Generated ToC * Clean up MUST and SHOULD in the document. * adjust for bug in markdown-toc * Mark protocol as stable. Co-authored-by: Carlos Alberto Cortez <calberto.cortez@gmail.com>
The Log SIG discussed and made a decision to declare Log data model and Log part of OTLP Beta. (See SIG meeting notes here https://docs.google.com/document/d/1cX5fWXyWqVVzYHSFUymYUfWxUK5hT97gc23w595LmdM/edit#heading=h.28y76as82bvu)
Related to open-telemetry/opentelemetry-specification#1816 Fixes open-telemetry/opentelemetry-specification#1835 Some historical context: we wanted to make grpc and http use the same port and we had an open issue in the Collector to do so: open-telemetry/opentelemetry-collector#1256 The conclusion is that there are technical hurdles that make it unfeasible: open-telemetry/opentelemetry-collector#1256 (comment) Because of that we need to keep grpc and http ports separate. This means we need to change the spec to say that otlp/http uses port 4318. Once this PR is merged we will also need to submit for port 4318 registration with IANA like we did previously with port 4317: open-telemetry/opentelemetry-specification#1148 (comment)
Apparently there is new evidence that one port may work after all: https://github.com/open-telemetry/opentelemetry-collector/pull/3765/files This reverts commit cce1d5996873de38a68e05eafa4d5e224df9b8f1 until we have the final decision about the ability to have a single port, see: open-telemetry/opentelemetry-specification#1846 (comment)
Related to open-telemetry/opentelemetry-specification#1816 Fixes open-telemetry/opentelemetry-specification#1835 Fixes open-telemetry/opentelemetry-specification#1920 Some historical context: we wanted to make grpc and http use the same port and we had an open issue in the Collector to do so: open-telemetry/opentelemetry-collector#1256 The conclusion is that there are technical hurdles that make it unfeasible: open-telemetry/opentelemetry-collector#1256 (comment) Because of that we need to keep grpc and http ports separate. This means we need to change the spec to say that otlp/http uses port 4318. Once this PR is merged we will also need to submit for port 4318 registration with IANA like we did previously with port 4317: open-telemetry/opentelemetry-specification#1148 (comment) There was also a draft PR to merge the ports in the Collector but it was not completed: open-telemetry/opentelemetry-collector#3765 Note that this change was initially submitted in PR open-telemetry/opentelemetry-specification#1839 and then reverted in PR open-telemetry/opentelemetry-specification#1847 because we hoped that the merging could be successfully done. We believe that we should no longer pursue this and should consider the ports separate from now on. Note 2: we consider this a spec bug fix (spec was impossible to implement), rather than a functionality change, that's why we believe this is an allowed change.
* Fix wrong spelling in README.md Fix wrong spelling in README.md * Polish document content
This is a follow up to [OTEP 239: Introduces Profiling Data Model v2](open-telemetry/oteps#239) The main motivation behind this PR is that this will allow us to start experimenting with the profiles proto in opentelemetry-collector. I marked the profiles part as `Experimental` to indicate that this is not a final version of the data model. I copied the proto from the OTEP, and moved `pprofextended.proto` from `profiles/v1/alternatives/pprofextended.proto` to just `profiles/v1/pprofextended.proto`. I did this because I figured we no longer have alternative representations and this will reduce confusion for people outside of Profiling SIG. The rest of the proto stayed the same. I tested this file with a collector fork and I it compiles properly.
The document is outdated. We are now using Github's release feature. Resolves open-telemetry#508 Co-authored-by: Armin Ruech <7052238+arminru@users.noreply.github.com>
…emetry#558) Gah, we put java_package on one file but not the other. Oops. Apparently the opentelementry-proto-java artifact can't be consumed by opentelemetry-java without this.
… between Sample and Link should be n-1 (open-telemetry#564)
Co-authored-by: Armin Ruech <7052238+arminru@users.noreply.github.com>
In the `Protocol Details` section the spec states that it defines how OTLP is implemented over grpc and **http 1.1**, but later in the document in section `OTLP/HTTP` it states "Implementations MAY use HTTP/1.1 or HTTP/2 transports." This may confuse end readers and I suggest that it is easier to drop the 1.1 in the Protocol Details section.
With `Mapping.filename`, `Function.name`, `Label.key` and others the type of the index into the string table is always of type `int64`. For consistency align the type of `Location.type_index`, which is also an index into the string table, to `int64`. Co-authored-by: Tigran Najaryan <4194920+tigrannajaryan@users.noreply.github.com>
…etry#576) The Profiles protocol is still experimental. Exclude it from the breaking-changes check as such changes are expected at this stage. Signed-off-by: Florian Lehner <florian.lehner@elastic.co>
The value for Sample.stacktrace_id_index is hardly defined except for its length. It is also not defined how this value should be generated, interpreted and could be used. Some kind of stacktrace_id can be most efficient in stateful protocols, where transmitting duplicate information should be avoided. As the OTel Profiling protocol is a stateless protocol, this element can be droped.
open-telemetry#576 excluded the profiles definition from breaking changes. But the entire profiles protocol is in development and can have breaking changes, including the gRPC service.
With open-telemetry/semantic-conventions#1188 the semantic convention for Profiles that introduces frame types and well known values is about to be merged. Therefore, `Location.type_index` is no longer needed. FYI: @open-telemetry/profiling-maintainers Co-authored-by: Tigran Najaryan <4194920+tigrannajaryan@users.noreply.github.com>
Drop `Sample.label` in favor of `Sample.attributes`. While `label` and `attributes` fulfill the same purpose, it is not defined which one choose for which information. Therefore, drop `label` in favor of `attributes`. FYI: @open-telemetry/profiling-maintainers spec:profiles Co-authored-by: Tigran Najaryan <4194920+tigrannajaryan@users.noreply.github.com>
Drop `BuildIdKind` in favor of its semantic convention as defined in open-telemetry/semantic-conventions#1329. Co-authored-by: Tigran Najaryan <4194920+tigrannajaryan@users.noreply.github.com>
As discussed in open-telemetry#582 (comment), this renames the `v1experimental` version for profiles to `v1development`. I've also taken this opportunity to update the versioning and stability link.
As commented in [0] and discussed in the OTel Profiling SIG meeting, there are situations where a main binary for a Profile can not be identified. For these cases mark the field optional. FYI: @brancz @petethepig @open-telemetry/profiling-maintainers [0]: open-telemetry#534 (comment)
felixge
force-pushed
the
profiling-signal
branch
from
0c3571c to
7d85bba
Compare
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.
No description provided.