opentelemetry-erlang icon indicating copy to clipboard operation
opentelemetry-erlang copied to clipboard

Bump opentelemetry_api from 1.2.1 to 1.2.2 breaks

Open epinault opened this issue 1 year ago • 14 comments

I am trying to apply a patch but my build get this error

what changed that could cause this? 1.2.1 worked fine. So something underlying is breaking now this is with Elixir 1.14 and OTP 25

7:03:44.100 [notice] Application opentelemetry exited: exited in: :opentelemetry_app.start(:normal, [])
    ** (EXIT) an exception was raised:
        ** (UndefinedFunctionError) function :otel_tracer_provider_sup.start/2 is undefined or private
            (opentelemetry 1.3.0) :otel_tracer_provider_sup.start(:global, %{attribute_count_limit: 128, attribute_per_event_limit: 128, attribute_per_link_limit: 128, attribute_value_length_limit: :infinity, bsp_exporting_timeout_ms: :undefined, bsp_max_queue_size: :undefined, bsp_scheduled_delay_ms: :undefined, create_application_tracers: true, deny_list: [], event_count_limit: 128, id_generator: :otel_id_generator, link_count_limit: 128, log_level: :info, metrics_exporter: {:opentelemetry_exporter, %{}}, processors: [otel_batch_processor: %{exporter: {:opentelemetry_exporter, %{}}, exporting_timeout_ms: 30000, max_queue_size: 2048, scheduled_delay_ms: 5000}], readers: [], register_loaded_applications: :undefined, resource_detector_timeout: 5000, resource_detectors: [:otel_resource_env_var, :otel_resource_app_env], sampler: {:parent_based, %{root: :always_on}}, ssp_exporting_timeout_ms: :undefined, sweeper: %{interval: 600000, span_ttl: 1800000, storage_size: :infinity, strategy: :drop}, text_map_propagators: [:trace_context, :baggage], traces_exporter: {:opentelemetry_exporter, %{}}, views: []})
            (opentelemetry 1.3.0) /code/deps/opentelemetry/src/opentelemetry_app.erl:39: :opentelemetry_app.start/2
            (kernel 8.5.4) application_master.erl:293: :application_master.start_it_old/4
            ```

epinault avatar Aug 21 '23 17:08 epinault

What SDK version are you using?

tsloughter avatar Aug 21 '23 21:08 tsloughter

what do you mean by SDK? what the package you are looking for?

epinault avatar Aug 21 '23 21:08 epinault

opentelemetry

tsloughter avatar Aug 21 '23 22:08 tsloughter

Same here. opentelemetry version is 1.2.1

smoggach-nl avatar Aug 22 '23 21:08 smoggach-nl

Pls try opentelemetry 1.3.1

tsloughter avatar Aug 22 '23 21:08 tsloughter

it works with that version of opentelemetry 👍

smoggach-nl avatar Aug 22 '23 21:08 smoggach-nl

same here, works with 1.3.1. Problem is tooling like Dependabot had 2 package changes so seems that it caused the problem .Once I upgraded the 1.3.1 and rebased the other one , it was all good

epinault avatar Aug 22 '23 22:08 epinault

For context. Updating both does fix the issue, but it's tricky with automation tools like dependabot, unfortunately.

@tsloughter if possible, maybe a good move is to retire the API 1.2.2 and introduce a new version which forces minimal version on the SDK. That should help, I assume.

andrewhr avatar Aug 23 '23 16:08 andrewhr

The API doesn't and can't depend on the SDK.

tsloughter avatar Aug 23 '23 17:08 tsloughter

Would it have worked out if I had properly bumped the version numbers as minors instead of patches?

tsloughter avatar Aug 23 '23 17:08 tsloughter

Not that it would have forced the SDK upgrade to be included but it maybe would have been handled different by tooling?

tsloughter avatar Aug 23 '23 17:08 tsloughter

The API doesn't and can't depend on the SDK.

Right, I completely forgot about that!

Would it have worked out if I had properly bumped the version numbers as minors instead of patches?

I think everyone just assumes the library is 100% semver, which in here wasn't the case. So minor bump would not help :/

andrewhr avatar Aug 23 '23 22:08 andrewhr

Yea. "semver". Definitely want to follow it as much as possible, and believe the Otel project requires it, but mistakes are made.

We keep backwards compatible functionality throughout the API to follow semver, the issue with my thinking here was that it is a call I thought of as only the API made to the SDK and not by users, so it wouldn't be noticed. But that doesn't work in this situation where the API doesn't actually depend on the SDK so it can't define a version constraint :)

The function that actually calls this code, start_tracer_provider was deprecated instead of removed for this very semver reason but apparently I failed at actually keeping it backwards compatible, haha.

Sorry about the issue, clearly need some testing setup that will verify the new API works with older SDKs.

tsloughter avatar Aug 23 '23 23:08 tsloughter

no worries . it s all fixed up for now for us. And yea dependabot can be tricky sometime for some packag

and hello @andrewhr :)

epinault avatar Aug 25 '23 15:08 epinault