Incorrect default url for OTLP HTTP

[dyladan]

While doing our due diligence before the 1.0 OTLP Trace Exporter release for JS we noticed that not all exporter implementations follow spec.

Spec states OTLP HTTP should use http://localhost:4318 by default.

Ruby is using https://localhost:4318/v1/traces (https should be http): link

[arielvalentin]

Thanks for the report @dyladan!

Do you have availability to open a PR with a fix?

[fbogsany]

open-telemetry/opentelemetry-specification#2014 changed the default from https to http.

[fbogsany]

There are probably other discrepancies in the exporter configuration we should look at. I don't think we've kept a close eye on the spec evolution since the original implementation of the exporter.

For example, there is some remaining work from #277 (comment) to rename the gem and add support for the OTEL_EXPORTER_OTLP_PROTOCOL environment variable.

[fbogsany]

Correct handling of OTEL_EXPORTER_OTLP_ENDPOINT vs OTEL_EXPORTER_OTLP_TRACES_ENDPOINT is a source of confusion. While the default value for OTEL_EXPORTER_OTLP_ENDPOINT is specced as http://localhost:4318, the signal specific path (v1/traces) is required to be appended to that value to produce the URL to be used by the exporter. The default value from

opentelemetry-ruby/exporter/otlp/lib/opentelemetry/exporter/otlp/exporter.rb

Line 47 in 46020cc

def initialize(endpoint: config_opt('OTEL_EXPORTER_OTLP_TRACES_ENDPOINT', 'OTEL_EXPORTER_OTLP_ENDPOINT', default: 'https://localhost:4318/v1/traces'), # rubocop:disable Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity

passes unmodified through

opentelemetry-ruby/exporter/otlp/lib/opentelemetry/exporter/otlp/exporter.rb

Lines 57 to 61 in 46020cc

	@uri = if endpoint == ENV['OTEL_EXPORTER_OTLP_ENDPOINT']
	URI("#{endpoint}/v1/traces")
	else
	URI(endpoint)
	end

My reading of the spec is that "default value" implies "default value assumed for OTEL_EXPORTER_OTLP_ENDPOINT if neither environment variable is present".

We could potentially restructure the code to make spec conformance clearer. Spec conformance for explicitly set OTEL_EXPORTER_OTLP_ENDPOINT is asserted here:

opentelemetry-ruby/exporter/otlp/test/opentelemetry/exporter/otlp/exporter_test.rb

Lines 65 to 84 in a8a596f

	it 'sets parameters from the environment' do
	exp = with_env('OTEL_EXPORTER_OTLP_ENDPOINT' => 'http://localhost:1234',
	'OTEL_EXPORTER_OTLP_CERTIFICATE' => '/foo/bar',
	'OTEL_EXPORTER_OTLP_HEADERS' => 'a=b,c=d',
	'OTEL_EXPORTER_OTLP_COMPRESSION' => 'gzip',
	'OTEL_RUBY_EXPORTER_OTLP_SSL_VERIFY_NONE' => 'true',
	'OTEL_EXPORTER_OTLP_TIMEOUT' => '11') do
	OpenTelemetry::Exporter::OTLP::Exporter.new
	end
	_(exp.instance_variable_get(:@headers)).must_equal('a' => 'b', 'c' => 'd')
	_(exp.instance_variable_get(:@timeout)).must_equal 11.0
	_(exp.instance_variable_get(:@path)).must_equal '/v1/traces'
	_(exp.instance_variable_get(:@compression)).must_equal 'gzip'
	http = exp.instance_variable_get(:@http)
	_(http.ca_file).must_equal '/foo/bar'
	_(http.use_ssl?).must_equal false
	_(http.address).must_equal 'localhost'
	_(http.verify_mode).must_equal OpenSSL::SSL::VERIFY_NONE
	_(http.port).must_equal 1234
	end

and spec conformance for the default OTEL_EXPORTER_OTLP_ENDPOINT value is asserted here:

opentelemetry-ruby/exporter/otlp/test/opentelemetry/exporter/otlp/exporter_test.rb

Lines 14 to 27 in a8a596f

	it 'initializes with defaults' do
	exp = OpenTelemetry::Exporter::OTLP::Exporter.new
	_(exp).wont_be_nil
	_(exp.instance_variable_get(:@headers)).must_be_empty
	_(exp.instance_variable_get(:@timeout)).must_equal 10.0
	_(exp.instance_variable_get(:@path)).must_equal '/v1/traces'
	_(exp.instance_variable_get(:@compression)).must_equal 'gzip'
	http = exp.instance_variable_get(:@http)
	_(http.ca_file).must_be_nil
	_(http.use_ssl?).must_equal true
	_(http.address).must_equal 'localhost'
	_(http.verify_mode).must_equal OpenSSL::SSL::VERIFY_PEER
	_(http.port).must_equal 4318
	end

although as mentioned, we haven't implemented the https -> http change from open-telemetry/opentelemetry-specification#2014

[fbogsany]

I think the only change required here is to change the default to http.