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

Consolidate OTLP variables to use TRACES / METRICS to describe talking… #1362

Merged
merged 9 commits into from
Jan 27, 2021

Conversation

anuraaga
Copy link
Contributor

… to TraceService / MetricsService

Figured we need a PR to try to knock these out :)

Fixes #1352
Fixes #1353

Changes

Environment variables for configuring OTLP exporters are changed from SPAN / METRIC to TRACE / METRICS. This is to correspond to the services being connected to, TraceService and MetricsService. It seems natural, and leaves out ambiguity by just matching up with the service names. Hopefully these variables won't be used all that much anyways in favor of the polyglot variables.

@anuraaga anuraaga requested review from a team January 22, 2021 05:45
@carlosalberto
Copy link
Contributor

I'm a little bit inclined to go with SPAN instead of TRACE (as, I think, all SIGs already use SpanExporter, just as the Specification itself does, as opposed to TraceExporter). Not a strong opinion, though.

@tedsuo
Copy link
Contributor

tedsuo commented Jan 25, 2021

We discussed in the Maintainers meeting, and chose TRACE.

Copy link
Member

@reyang reyang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

Anuraag Agrawal added 3 commits January 26, 2021 12:10
@carlosalberto
Copy link
Contributor

@open-telemetry/specs-metrics-approvers Please somebody give us a blessing here, as Metrics is also updated ;)

Copy link
Member

@tigrannajaryan tigrannajaryan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

specification/protocol/exporter.md Outdated Show resolved Hide resolved
specification/protocol/exporter.md Outdated Show resolved Hide resolved
specification/protocol/exporter.md Outdated Show resolved Hide resolved
specification/protocol/exporter.md Outdated Show resolved Hide resolved
specification/protocol/exporter.md Outdated Show resolved Hide resolved
specification/protocol/exporter.md Outdated Show resolved Hide resolved
specification/protocol/exporter.md Outdated Show resolved Hide resolved
specification/protocol/exporter.md Outdated Show resolved Hide resolved
@anuraaga
Copy link
Contributor Author

OTLP uses plural traces, metrics and logs for consistency:

@tigrannajaryan Really? I only see trace in the package, file, and service name so think it is closer

https://github.com/open-telemetry/opentelemetry-proto/tree/master/opentelemetry/proto

@tigrannajaryan
Copy link
Member

OTLP uses plural traces, metrics and logs for consistency:

@tigrannajaryan Really? I only see trace in the package, file, and service name so think it is closer

https://github.com/open-telemetry/opentelemetry-proto/tree/master/opentelemetry/proto

We cannot change those name since it breaks compatibility and they are already declared stable.

However, as far as I remember this was discussed and a decision was made that we should use plural for all signals going forward where we can. Collector codebase uses plural traces everywhere. OTLP/HTTP uses /v1/traces as the default URL.

@carlosalberto
Copy link
Contributor

After a second look, agreed that we should go for TRACES, as keeping all signals plural is easier to remember IMHO (besides the compatibility reasons @tigrannajaryan gave).

Copy link
Member

@bogdandrutu bogdandrutu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 for @tigrannajaryan comments

Anuraag Agrawal added 2 commits January 27, 2021 16:39
…emetry-specification into trace-metrics-variables
Copy link
Contributor Author

@anuraaga anuraaga left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated

@bogdandrutu bogdandrutu changed the title Consolidate OTLP variables to use TRACE / METRICS to describe talking… Consolidate OTLP variables to use TRACES / METRICS to describe talking… Jan 27, 2021
@bogdandrutu bogdandrutu merged commit 493a8e8 into open-telemetry:master Jan 27, 2021
carlosalberto added a commit to carlosalberto/opentelemetry-specification that referenced this pull request Oct 31, 2024
…g… (open-telemetry#1362)

* Consolidate OTLP variables to use TRACE / METRICS to describe talking to TraceService / MetricsService

* CHANGELOG

* Quote

* S

* Update CHANGELOG.md

Co-authored-by: Carlos Alberto Cortez <calberto.cortez@gmail.com>
Co-authored-by: Bogdan Drutu <lazy@splunk.com>
Co-authored-by: Bogdan Drutu <bogdandrutu@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
8 participants