Split out OTLP gRPC vs. HTTP endpoint configuration #1729
Conversation
There was a problem hiding this comment.
Thanks @alanwest
Please also add a changelog entry.
I believe this is a non-breaking change since it makes Endpoint processing more permissive. Every endpoint value that was acceptable previously is still acceptable. Some values which were previously not accepted and would result in an error will be now accepted. I think we can treat this as a non-breaking change.
specification/protocol/exporter.md
Outdated
| | Endpoint | Target to which the exporter is going to send spans or metrics. The endpoint MUST be a valid URL with scheme (http or https) and host, and MAY contain a port and path. A scheme of https indicates a secure connection. When using `OTEL_EXPORTER_OTLP_ENDPOINT` with OTLP/HTTP, exporters SHOULD follow the collector convention of appending the version and signal to the path (e.g. `v1/traces` or `v1/metrics`). The per-signal endpoint configuration options take precedence and can be used to override this behavior. See the [OTLP Specification][otlphttp-req] for more details. | `https://localhost:4317` | `OTEL_EXPORTER_OTLP_ENDPOINT` `OTEL_EXPORTER_OTLP_TRACES_ENDPOINT` `OTEL_EXPORTER_OTLP_METRICS_ENDPOINT` | | ||
| | Endpoint (OTLP/HTTP) | Target to which the exporter is going to send spans or metrics. The endpoint MUST be a valid URL with scheme (http or https) and host, and MAY contain a port and path. A scheme of https indicates a secure connection. When using `OTEL_EXPORTER_OTLP_ENDPOINT`, exporters SHOULD follow the collector convention of appending the version and signal to the path (e.g. `v1/traces` or `v1/metrics`). The per-signal endpoint configuration options take precedence and can be used to override this behavior. See the [OTLP Specification][otlphttp-req] for more details. | `https://localhost:4317` | `OTEL_EXPORTER_OTLP_ENDPOINT` `OTEL_EXPORTER_OTLP_TRACES_ENDPOINT` `OTEL_EXPORTER_OTLP_METRICS_ENDPOINT` | | ||
| | Endpoint (OTLP/gRPC) | Target to which the exporter is going to send spans or metrics. The endpoint MUST accept a URL with or without a scheme. If it includes a scheme (http or https), a scheme of https indicates a secure connection. | `https://localhost:4317` | `OTEL_EXPORTER_OTLP_ENDPOINT` `OTEL_EXPORTER_OTLP_TRACES_ENDPOINT` `OTEL_EXPORTER_OTLP_METRICS_ENDPOINT` | | ||
| | Insecure | Whether to enable client transport security for the exporter's `grpc` connection. Only applies to OTLP/gRPC when a scheme is not included in the `endpoint`. OTLP/HTTP always uses the scheme provided for the endpoint. | `false` | `OTEL_EXPORTER_OTLP_INSECURE` `OTEL_EXPORTER_OTLP_SPAN_INSECURE` `OTEL_EXPORTER_OTLP_METRIC_INSECURE` | |
There was a problem hiding this comment.
Does this mean SDKs all have to support this variable? I'd prefer not to support it - we already have a method for picking secure / insecure in the endpoint scheme, having another way of doing the same thing is extra documentation / support overhead (not to mention bringing back this problematic word insecure which security scanners don't like much). If the idea is to allow either (no-scheme url + insecure flag) or (scheme url) than that's ok but don't see that mentioned in this wording.
There was a problem hiding this comment.
Another point is that it's always been quite mysterious when host:port defaults to TLS - I don't think that matches most users' expectations, it's just a security push by us I think. The schemed URLs solved this problem nicely, if allowing non-schemed URLs I really recommend the default being insecure, I think it matches most users' expectations (even curl google.com will hit HTTP, not HTTPS, this is what people think IMO).
There was a problem hiding this comment.
Does this mean SDKs all have to support this variable?
Yes, my thought was that SDKs and collector would support the insecure setting.
I'd prefer not to support it - we already have a method for picking secure / insecure in the endpoint scheme, having another way of doing the same thing is extra documentation / support overhead (not to mention bringing back this problematic word
insecurewhich security scanners don't like much).
I agree supporting it is not a thrilling idea. Coming from .NET, the client we're using for the OTLP exporter requires a scheme anyways. Though my understanding is that quite a few gRPC client libraries for other languages support declaring an endpoint without a scheme and that they default to using the dns scheme and a secure channel is the default.
The insecure wording seems to be inspired by gRPC libraries themselves - here are some examples for setting up an insecure channel in various languages.
If the idea is to allow either (no-scheme url + insecure flag) or (scheme url) than that's ok but don't see that mentioned in this wording.
This is the idea. I attempted to capture that with my wording:
- "Only applies to OTLP/gRPC when a scheme is not included in the
endpoint."
Another point is that it's always been quite mysterious when host:port defaults to TLS - I don't think that matches most users' expectations, it's just a security push by us I think.
I'm not an expert, though I'm not entirely certain that it was just a security push on the part of OpenTelemetry. My read of the examples I linked to above seems to suggest that secure channels are considered the default by most gRPC libraries.
All this said, I had also hoped that scheme alone could solve this issue, but it seems that declaring a URI without a scheme is common with many libraries. In speaking with the collector SIG they would like to maintain the ability to configure without a scheme. Go SDK also still supports the insecure config setting.
So I guess it's a question of how important it is we drive consistency across the language SDKs. The inconsistency has been a point of confusion to some.
There was a problem hiding this comment.
like to maintain the ability to configure without a scheme. Go SDK also still supports the
insecureconfig setting
Do they want to configure without scheme but still use TLS?
There was a problem hiding this comment.
I'm not sure if it's required for collector and SDK to be consistent here as I think these variables are mostly for SDK.
SDKs will always have extra knobs compared to what's in the spec. Usually they're language specific but it doesn't seem terrible if one SDK supports insecure flag if they want to. For Java though we've had no user issues related to this flag and it seems to work well, so I'm thinking don't fix what isn't broken. I can't see any chance of improvement by requiring support of the new flag but a chance of more user confusion when having multiple flags.
There was a problem hiding this comment.
Do they want to configure without scheme but still use TLS?
Correct, TLS is the default in Go.
Go looks something like this
driver := otlpgrpc.NewDriver(
otlpgrpc.WithInsecure(), // This option is required if you do not want TLS otherwise TLS is the default
otlpgrpc.WithEndpoint("localhost:4317"), // To my knowledge, including a scheme here does not work
)
exp, err := otlp.NewExporter(ctx, driver)On the other hand, Go does support a scheme when configuration is done via the OTEL_EXPORTER_OTLP_ENDPOINT environment variable. The scheme http implies the WithInsecure option.
There was a problem hiding this comment.
SDKs will always have extra knobs compared to what's in the spec. Usually they're language specific but it doesn't seem terrible if one SDK supports insecure flag if they want to.
This stance seems reasonable.
Perhaps at a minimum do you think it makes sense to say for OTLP/gRPC:
- SDKs MUST support endpoint with configured with a scheme and the scheme
httpsindicates use TLS. - SDKs MAY support an endpoint without a scheme but if they do then an additional
insecureconfiguration is required.
First point effectively maintains status quo. Second point enables the flexibility for SDKs and requires a level of consistency when adopting this.
There was a problem hiding this comment.
Yup I have no problem with text related to an insecure flag if it's optional.
There was a problem hiding this comment.
I've clarified things so that implementing the insecure flag is optional if the SDK does not allow for an endpoint to be configured without a scheme.
…int-configuration
…nwest/opentelemetry-specification into alanwest/otlp-endpoint-configuration
|
I feel like this needs more eyes. @open-telemetry/specs-approvers please review. |
|
@open-telemetry/specs-approvers Please do review/approve ;) |
specification/protocol/exporter.md
Outdated
| @@ -10,7 +10,9 @@ The following configuration options MUST be available to configure the OTLP expo | |||
|
|
|||
| | Configuration Option | Description | Default | Env variable | | |||
| | -------------------- | ------------------------------------------------------------ | ----------------- | ------------------------------------------------------------ | | |||
| | Endpoint | Target to which the exporter is going to send spans or metrics. The endpoint MUST be a valid URL with scheme (http or https) and host, and MAY contain a port and path. A scheme of https indicates a secure connection. When using `OTEL_EXPORTER_OTLP_ENDPOINT` with OTLP/HTTP, exporters SHOULD follow the collector convention of appending the version and signal to the path (e.g. `v1/traces` or `v1/metrics`). The per-signal endpoint configuration options take precedence and can be used to override this behavior. See the [OTLP Specification][otlphttp-req] for more details. | `https://localhost:4317` | `OTEL_EXPORTER_OTLP_ENDPOINT` `OTEL_EXPORTER_OTLP_TRACES_ENDPOINT` `OTEL_EXPORTER_OTLP_METRICS_ENDPOINT` | | |||
| | Endpoint (OTLP/HTTP) | Target to which the exporter is going to send spans or metrics. The endpoint MUST be a valid URL with scheme (http or https) and host, and MAY contain a port and path. A scheme of https indicates a secure connection. When using `OTEL_EXPORTER_OTLP_ENDPOINT`, exporters SHOULD follow the collector convention of appending the version and signal to the path (e.g. `v1/traces` or `v1/metrics`). The per-signal endpoint configuration options take precedence and can be used to override this behavior. See the [OTLP Specification][otlphttp-req] for more details. | `https://localhost:4317` | `OTEL_EXPORTER_OTLP_ENDPOINT` `OTEL_EXPORTER_OTLP_TRACES_ENDPOINT` `OTEL_EXPORTER_OTLP_METRICS_ENDPOINT` | | |||
| | Endpoint (OTLP/gRPC) | Target to which the exporter is going to send spans or metrics. The endpoint MUST accept a URL with a scheme and MAY accept a URL without a scheme. If it includes a scheme (http or https), a scheme of https indicates a secure connection. If it does not include a scheme then the `insecure` configuration option is used to determine whether a secure connection should be used. | `https://localhost:4317` | `OTEL_EXPORTER_OTLP_ENDPOINT` `OTEL_EXPORTER_OTLP_TRACES_ENDPOINT` `OTEL_EXPORTER_OTLP_METRICS_ENDPOINT` | | |||
There was a problem hiding this comment.
For gRPC it is my understanding that the scheme is used to determine the address resolution method and not the transport protocol since the transport protocol is always HTTP/2, which requires TLS unless explicitly configured otherwise with and indication that insecure connections are acceptable.
Valid scheme values for gRPC are dns, unix, unix-abstract, ipv4, and ipv6. dns is the default value if no scheme is specified.
Using http and https as a shorthand for dns-with-insecure and dns may be acceptable, but we should be very explicit about what we're doing and how client implementations should treat otherwise invalid gRPC endpoint names. We should also make clear that the other gRPC-supported schemes are valid.
There was a problem hiding this comment.
This is very important point IMO. If gRPC already assigns some meaning to schema part of their URLs then won't it be confusing if we offer completely different schemas with completely different meaning for "gRPC URLs"?
There was a problem hiding this comment.
@alanwest You use the same environment variables for both /HTTP and /gRPC options. How should SDK understand which protocol to use?
There was a problem hiding this comment.
You use the same environment variables for both /HTTP and /gRPC options. How should SDK understand which protocol to use?
I left them the same since it has always been the case that the same environment variables are to be used for both HTTP and gRPC exporters.
To my knowledge, some languages only support gRPC at the moment. For languages that support exporting via both gRPC and HTTP it seems common that these are shipped as two distinct components.
I do not know the history here, but the specification does mention a OTEL_EXPORTER_OTLP_PROTOCOL environment variable with the note "We reserve the following environment variables for configuration of protocols in the future".
I imagine that the OTEL_EXPORTER_OTLP_PROTOCOL variable would be necessary if a language SDK decided to develop a single component that could be configured to export either by HTTP or gRPC.
There was a problem hiding this comment.
Using
httpandhttpsas a shorthand fordns-with-insecureanddnsmay be acceptable, but we should be very explicit about what we're doing and how client implementations should treat otherwise invalid gRPC endpoint names. We should also make clear that the other gRPC-supported schemes are valid.
Agreed there's some room to make this more clear - I took a stab at new wording, but I'm open to any suggestions for making this any more clear. See this commit.
I've attempted to keep things as client implementation agnostic as possible yet striving to be very clear. Client implementations differ in the schemes they support. As a point of contrast to the valid scheme values for gRPC you pointed out, the .NET gRPC library, for example, requires including a scheme of http or https - that is, it doesn't really have a notion of defaulting to the dns scheme and in fact if you explicitly add dns:// it doesn't work.
That said, I think it's safe to require OTel SDKs to require the ability to declare http(s) - I think the risk is low that we would stomp on the semantics of any specific client implementation.
There was a problem hiding this comment.
This is much better, I think. That said, the handling of insecure still feels weird. None of the gRPC standard schemes convey any information about whether TLS enforcement is necessary. It seems wrong to say that insecure should only be honored when no scheme is present. The address resolution scheme and TLS enforcement are distinct concerns for gRPC (and even for HTTP where a service may initiate TLS with an untrusted certificate and the user wants to ignore that).
There was a problem hiding this comment.
That said, the handling of insecure still feels weird.
Yep agreed, excellent callout. I've cleaned up the wording for the insecure option here.
I've attempted again to remain as flexible and non-prescriptive as possible given the variety of client implementations some of which do not have a need for a separate insecure setting.
specification/protocol/exporter.md
Outdated
| @@ -10,7 +10,9 @@ The following configuration options MUST be available to configure the OTLP expo | |||
|
|
|||
| | Configuration Option | Description | Default | Env variable | | |||
| | -------------------- | ------------------------------------------------------------ | ----------------- | ------------------------------------------------------------ | | |||
| | Endpoint | Target to which the exporter is going to send spans or metrics. The endpoint MUST be a valid URL with scheme (http or https) and host, and MAY contain a port and path. A scheme of https indicates a secure connection. When using `OTEL_EXPORTER_OTLP_ENDPOINT` with OTLP/HTTP, exporters SHOULD follow the collector convention of appending the version and signal to the path (e.g. `v1/traces` or `v1/metrics`). The per-signal endpoint configuration options take precedence and can be used to override this behavior. See the [OTLP Specification][otlphttp-req] for more details. | `https://localhost:4317` | `OTEL_EXPORTER_OTLP_ENDPOINT` `OTEL_EXPORTER_OTLP_TRACES_ENDPOINT` `OTEL_EXPORTER_OTLP_METRICS_ENDPOINT` | | |||
| | Endpoint (OTLP/HTTP) | Target to which the exporter is going to send spans or metrics. The endpoint MUST be a valid URL with scheme (http or https) and host, and MAY contain a port and path. A scheme of https indicates a secure connection. When using `OTEL_EXPORTER_OTLP_ENDPOINT`, exporters SHOULD follow the collector convention of appending the version and signal to the path (e.g. `v1/traces` or `v1/metrics`). The per-signal endpoint configuration options take precedence and can be used to override this behavior. See the [OTLP Specification][otlphttp-req] for more details. | `https://localhost:4317` | `OTEL_EXPORTER_OTLP_ENDPOINT` `OTEL_EXPORTER_OTLP_TRACES_ENDPOINT` `OTEL_EXPORTER_OTLP_METRICS_ENDPOINT` | | |||
| | Endpoint (OTLP/gRPC) | Target to which the exporter is going to send spans or metrics. The endpoint MUST accept a URL with a scheme and MAY accept a URL without a scheme. If it includes a scheme (http or https), a scheme of https indicates a secure connection. If it does not include a scheme then the `insecure` configuration option is used to determine whether a secure connection should be used. | `https://localhost:4317` | `OTEL_EXPORTER_OTLP_ENDPOINT` `OTEL_EXPORTER_OTLP_TRACES_ENDPOINT` `OTEL_EXPORTER_OTLP_METRICS_ENDPOINT` | | |||
There was a problem hiding this comment.
This is very important point IMO. If gRPC already assigns some meaning to schema part of their URLs then won't it be confusing if we offer completely different schemas with completely different meaning for "gRPC URLs"?
specification/protocol/exporter.md
Outdated
| @@ -10,7 +10,9 @@ The following configuration options MUST be available to configure the OTLP expo | |||
|
|
|||
| | Configuration Option | Description | Default | Env variable | | |||
| | -------------------- | ------------------------------------------------------------ | ----------------- | ------------------------------------------------------------ | | |||
| | Endpoint | Target to which the exporter is going to send spans or metrics. The endpoint MUST be a valid URL with scheme (http or https) and host, and MAY contain a port and path. A scheme of https indicates a secure connection. When using `OTEL_EXPORTER_OTLP_ENDPOINT` with OTLP/HTTP, exporters SHOULD follow the collector convention of appending the version and signal to the path (e.g. `v1/traces` or `v1/metrics`). The per-signal endpoint configuration options take precedence and can be used to override this behavior. See the [OTLP Specification][otlphttp-req] for more details. | `https://localhost:4317` | `OTEL_EXPORTER_OTLP_ENDPOINT` `OTEL_EXPORTER_OTLP_TRACES_ENDPOINT` `OTEL_EXPORTER_OTLP_METRICS_ENDPOINT` | | |||
| | Endpoint (OTLP/HTTP) | Target to which the exporter is going to send spans or metrics. The endpoint MUST be a valid URL with scheme (http or https) and host, and MAY contain a port and path. A scheme of https indicates a secure connection. When using `OTEL_EXPORTER_OTLP_ENDPOINT`, exporters SHOULD follow the collector convention of appending the version and signal to the path (e.g. `v1/traces` or `v1/metrics`). The per-signal endpoint configuration options take precedence and can be used to override this behavior. See the [OTLP Specification][otlphttp-req] for more details. | `https://localhost:4317` | `OTEL_EXPORTER_OTLP_ENDPOINT` `OTEL_EXPORTER_OTLP_TRACES_ENDPOINT` `OTEL_EXPORTER_OTLP_METRICS_ENDPOINT` | | |||
| | Endpoint (OTLP/gRPC) | Target to which the exporter is going to send spans or metrics. The endpoint MUST accept a URL with a scheme and MAY accept a URL without a scheme. If it includes a scheme (http or https), a scheme of https indicates a secure connection. If it does not include a scheme then the `insecure` configuration option is used to determine whether a secure connection should be used. | `https://localhost:4317` | `OTEL_EXPORTER_OTLP_ENDPOINT` `OTEL_EXPORTER_OTLP_TRACES_ENDPOINT` `OTEL_EXPORTER_OTLP_METRICS_ENDPOINT` | | |||
There was a problem hiding this comment.
@alanwest You use the same environment variables for both /HTTP and /gRPC options. How should SDK understand which protocol to use?
Co-authored-by: Nikita Salnikov-Tarnovski <[email protected]>
|
This PR was marked stale due to lack of activity. It will be closed in 7 days. |
|
@alanwest please resolve the conflicts. |
|
This has been around for some time now, and no more reviews/issues were reporting, hence merging. |
otlpexporter endpoint configuration allows for an `http` or `https` scheme. The scheme is stripped off when instantiating the gRPC channel. A scheme of `https` indicates a secure channel. This aligns the collector with this hopefully soon-to-be-merged specification PR open-telemetry/opentelemetry-specification#1729 **Link to tracking Issue:** #2539 **Documentation:** Updated readme
Here's a can of worms some folks may have hoped would just go away 😄.
Background
The specification of the endpoint configuration for the OTLP exporter has caused some confusion among users due to inconsistencies with the implementations of the language SDKs as well as the collector.
Related issues
Prior context
In a prior version of the specification, the endpoint configuration did not mandate it be a URL with scheme included. There also used to be an
insecureconfiguration flag which was removed. See #1234 where these changes were made. It seems that these changes were originally intended for the benefit of making the configuration of OTLP/HTTP exporters more natural, yet the discussion on this PR seems to suggest that folks accepted that the changes would equally apply to OTLP/gRPC exporters.Existing implementations
I have studied a few of the OTLP/gRPC exporter implementations. Please correct me on any of the observations I have made - I am not an expert in all of these languages.
.NET and Java SDKs
httporhttps.insecureconfiguration option.Go SDK and Collector
address https://some-host:4317/: too many colons in addressOTEL_EXPORTER_OTLP_ENDPOINTenvironment variable. See Add support for OTEL_EXPORTER_OTLP_ENDPOINT according to the spec opentelemetry-go#1885.insecureconfiguration option no longer present in the specification.JS/Node
http,https,grpc, orgrpcs. Code hereChannelCredentials. TheChannelCredentialsare used here.Input from Collector SIG
Based on a conversation on this topic with folks (@tigrannajaryan @bogdandrutu @alolita) during the 5/26 collector SIG meeting, I am drafting this PR to reopen a discussion for how (and if) we wish to resolve these inconsistencies.
To summarize the discussion from 5/26 collector SIG meeting:
Changes
In an attempt to address 1) the desire for backwards compatibility and 2) the confusion brought on from the inconsistencies in the various implementations, here's the idea I'm toying with:
insecureflag only for use by the OTLP/gRPC exporter and only when a scheme is not present in the endpoint configuration.