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

Clarify TLS/Secure default behavior #2420

Closed
dyladan opened this issue Mar 17, 2022 · 7 comments · Fixed by #2476
Closed

Clarify TLS/Secure default behavior #2420

dyladan opened this issue Mar 17, 2022 · 7 comments · Fixed by #2476
Assignees
Labels
spec:protocol Related to the specification/protocol directory

Comments

@dyladan
Copy link
Member

dyladan commented Mar 17, 2022

The configuration options for the OTLP gRPC exporter are a little unclear what to do if no endpoint is configured. By a strict reading of the current spec it seems like a secure connection to localhost:4317 should be made, but it seems likely that certificate validation will fail in this case most of the time if a certificate is not supplied to the client. I would propose a secure connection is only the default here if a certificate is provided for validation, or we should specify that if no endpoint is provided TLS certificate validation should be disabled.

@tigrannajaryan
Copy link
Member

I believe the default connection to localhost should be insecure (I am not aware of any benefits of using encryption for localhost connections). If we agree on this then the spec needs to be clarified.

@dyladan
Copy link
Member Author

dyladan commented Apr 6, 2022

There are still a couple open questions in open-telemetry/opentelemetry-js#2827

Specifically:

  • if user sets http scheme and no other items (like providing credentials or env insecure settings) return insecure channel

This is not as clear. I'll ask Tigran. The spec says insecure is false by default, but http implies insecure.

  • if user wants to use all defaults, return secure channel

According to @tigrannajaryan open-telemetry/opentelemetry-specification#2420 it should be insecure to localhost. Tigran agrees spec should be clarified.

  • if user sets http scheme and env insecure = false, return secure channel

Yes. I think this will also be the answer for "user sets http scheme and no other items" but we'll wait for Tigran.

Would appreciate some clarification/guidance here.

@tigrannajaryan
Copy link
Member

  • if user sets http scheme and no other items (like providing credentials or env insecure settings) return insecure channel

This is not as clear. I'll ask Tigran. The spec says insecure is false by default, but http implies insecure.

The spec says that "insecure" option is ignored for OTLP/HTTP. So, with the default protocol of http/protobuf and default destination of http://localhost:4318 I believe there is no ambiguity. It means: use OTLP/HTTP, insecure.

@dyladan
Copy link
Member Author

dyladan commented Apr 6, 2022

This is specifically for the gRPC exporter

@tigrannajaryan
Copy link
Member

For gRPC the spec says:

Endpoint (OTLP/gRPC): Target to which the exporter is going to send spans or metrics. The endpoint SHOULD accept
 any form allowed by the underlying gRPC client implementation. Additionally, the endpoint MUST accept a URL with 
a scheme of either http or https. A scheme of https indicates a secure connection and takes precedence over the 
insecure configuration setting.

I think we need to modify it to say that if the "http" or "https" scheme is used in the endpoint then that takes precedence over the insecure configuration setting (so, not just the "https" as it says currently, but also "http").

I think that should remove the ambiguity.

@dyladan
Copy link
Member Author

dyladan commented Apr 7, 2022

So the insecure config is only used if there is no scheme in the URL?

@tigrannajaryan
Copy link
Member

So the insecure config is only used if there is no scheme in the URL?

Yes, I think so.

tigrannajaryan pushed a commit that referenced this issue Apr 12, 2022
Fixes #2420

## Changes

- Clarify the insecure option for gRPC exporters

Related issues

- open-telemetry/opentelemetry-js#2827
carlosalberto pushed a commit to carlosalberto/opentelemetry-specification that referenced this issue Oct 31, 2024
Fixes open-telemetry#2420

## Changes

- Clarify the insecure option for gRPC exporters

Related issues

- open-telemetry/opentelemetry-js#2827
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
spec:protocol Related to the specification/protocol directory
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants