Not easy to configure Exporters with block

[miry]

Here is an example of the code:

config.exporter = OpenTelemetry::Exporter.new(variant: "http") do |c|
  c.batch_latency = 1
end

it raises the error:

 $ crystal run --error-trace --debug otlp_http_exporter_sample.cr
In otlp_http_exporter_sample.cr:24:45

 24 | config.exporter = OpenTelemetry::Exporter.new(variant: "http") do |c|
                                                ^--
Error: instantiating 'OpenTelemetry::Exporter.class#new()'


In lib/opentelemetry-api/src/exporter.cr:19:38

 19 | @exporter = Exporter::Stdout.new do |obj|
                                   ^--
Error: instantiating 'OpenTelemetry::Exporter::Stdout.class#new()'


In lib/opentelemetry-api/src/exporter.cr:19:38

 19 | @exporter = Exporter::Stdout.new do |obj|
                                   ^--
Error: instantiating 'OpenTelemetry::Exporter::Stdout.class#new()'


In otlp_http_exporter_sample.cr:24:45

 24 | config.exporter = OpenTelemetry::Exporter.new(variant: "http") do |c|
                                                ^--
Error: instantiating 'OpenTelemetry::Exporter.class#new()'


In otlp_http_exporter_sample.cr:26:5

 26 | cc.batch_latency = 1
      ^-
Error: undefined local variable or method 'cc' for top-level

it makes sense that Stdout, because Stdout does not implement Batch processor.

Currently workaroud:

  config.exporter = OpenTelemetry::Exporter.new(variant: "http") do |c|
    cc = c.as(OpenTelemetry::Exporter::Http)
    cc.batch_latency = 1
  end

[wyhaines]

Yep.

The examples cover this, though they don't explain why. e.x.

OpenTelemetry.configure do |config|
  config.service_name = "Fibonacci Server"
  config.service_version = Fibonacci::VERSION
  config.exporter = OpenTelemetry::Exporter.new(variant: :http) do |exporter|
    exporter = exporter.as(OpenTelemetry::Exporter::Http)
    exporter.endpoint = "https://otlp.nr-data.net:4318/v1/traces"
    headers = HTTP::Headers.new
    headers["api-key"] = ENV["NEW_RELIC_LICENSE_KEY"]?.to_s
    exporter.headers = headers
  end
end

That as call is needed because, with exporters, we are dealing with classes that may be initialized differently, with different parameters, and getting those typing differences to all live together under a single call interface proved challenging. This was the best compromise that I could come up with at the time. This is also why the container Exporter class exists. It was the easiest way to solve the typing issues that I was encountering at the time.

There may well be a better solution, but I have not gone back into that part of the code to see if something better comes to mind yet. I am open to suggestions.

[wyhaines]

I have a fix. It may not be the best fix, but I have a fix.

Exporter::Base will receive a method_missing macro defined on it which will raise at runtime. This will allow assignment in the block without imposing a type restriction to the specific type of the object.

[miry]

@wyhaines what do you think also to support configure option via Hash argument? Example:

OpenTelemetry::Exporter.new(variant: :http, options: {"batch_latency": "1"})

It would potentialy cover the case of configuring Exporters via environment variables as well.

[wyhaines]

That gets deceptively tricky. Every exporter would have to have a method that can take an options hash and use it for configuration, and I'm not sure what the benefit would be. Keyword args do work, and I do think that they look better than options hashes.

Can you tell me what benefit that you see to hash based initialization?

[miry]

Can you tell me what benefit that you see to hash based initialization?

I think to not replace current one, but to add as seperate method.

Here is something that helped me in other projects:

• Allow dynamicaly setup settings. Example:

  options = JSON.parse(ENV.fetch("OTEL_EXPORTER_HTTP_OPTIONS", "{}"))
  if test?
    options["batch_latency"] = 12
  else
    options["batch_latency"] ||= 1
  end
  
  subject = OpenTelemetry::Exporter.new(:http, options)

• Make single line initialization:

  exporter = OpenTelemetry::Exporter.new(:http, {"option_foo": "bar"})

My proposal is not critical at all, because the solution you advised in #23 (comment) should cover Types' issue and would support minimum required solution.