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

Incompatibility betweeen opentelemetry_oban and opentelemetry_bandit #428

Open
michelboaventura opened this issue Dec 11, 2024 · 9 comments · May be fixed by #435
Open

Incompatibility betweeen opentelemetry_oban and opentelemetry_bandit #428

michelboaventura opened this issue Dec 11, 2024 · 9 comments · May be fixed by #435
Labels
bug Something isn't working

Comments

@michelboaventura
Copy link

Describe the bug
I have this on my mix.exs

{:opentelemetry, "~> 1.3"},
{:opentelemetry_api, "~> 1.2"},
{:opentelemetry_bandit, "~> 0.2"},
{:opentelemetry_ecto, "~> 1.2"},
{:opentelemetry_exporter, "~> 1.6"},
{:opentelemetry_oban, "~> 1.1"}

Then when I run mix deps.get I'm getting (formatted for better reading):

Resolving Hex dependencies...
Resolution completed in 0.471s
Because opentelemetry_bandit >= 0.2.0-rc.1 depends on opentelemetry_semantic_conventions ~> 1.27 and
opentelemetry_oban >= 1.1.0 depends on opentelemetry_semantic_conventions ~> 0.2,
opentelemetry_bandit >= 0.2.0-rc.1 is incompatible with opentelemetry_oban >= 1.1.0.
And because your app depends on opentelemetry_bandit ~> 0.2, opentelemetry_oban >= 1.1.0 is forbidden.
So, because your app depends on opentelemetry_oban ~> 1.1, version solving failed.
** (Mix) Hex dependency resolution failed

Expected behavior
I'd like to use both bandit and oban at the same time.

@michelboaventura michelboaventura added the bug Something isn't working label Dec 11, 2024
@anthonator
Copy link

We're seeing the same issue between opentelemtry_cowboy and opentelemetry_oban.

Because opentelemetry_cowboy >= 1.0.0-rc.1 depends on opentelemetry_semantic_conventions ~> 1.27 and
opentelemetry_oban >= 1.1.0 depends on opentelemetry_semantic_conventions ~> 0.2,
opentelemetry_cowboy >= 1.0.0-rc.1 is incompatible with opentelemetry_oban >= 1.1.0.
And because your app depends on opentelemetry_cowboy 1.0.0, opentelemetry_oban >= 1.1.0 is forbidden.
So, because your app depends on opentelemetry_oban 1.1.1, version solving failed.

@bryannaegele
Copy link
Collaborator

{:opentelemetry_semantic_conventions, "~> 1.27", override: true}

@danschultzer
Copy link

I’ve opened this PR to resolve this: #435

@davidjulien
Copy link

{:opentelemetry_semantic_conventions, "~> 1.27", override: true}

Hello @bryannaegele and other contributors!

I hope you're doing well! I wanted to reach out regarding the workaround you suggested with {:opentelemetry_semantic_conventions, "~> 1.27", override: true}. Unfortunately, this approach doesn't quite work for us since our opentelemetry dependencies are nested within a library that we include across several projects, and hex.pm doesn't support overrides within dependencies.

We're in a bit of a bind at the moment, as we're relying on the updates to these dependencies. The PRs targeting the upgrade of cowboy and oban to semantic conventions 1.27 are still awaiting review, and this has been holding us back for a few weeks now:

We understand everyone has a lot on their plate, especially around this time of year, and we truly appreciate all the efforts being made. If there's anything we can do to help expedite the process, please let us know. It would be a wonderful gift to wrap up this year!

Happy holidays! 🎄

@bryannaegele
Copy link
Collaborator

@davidjulien I'd like to get them merged but it requires a deep review of the spec to ensure the PR complies with the spec. I will drop a note to the PR authors to ask but these updates to 1.27 usually involve quite a bit more work than just bumping the dependency and calling it a day.

We try to rely on the codeowners to review PRs to their libraries which in this case are @whatyouhide and @tomtaylor for Broadway and @indrekj for Oban. Tristan and I just do not have the capacity to review every PR in a timely manner.

I'm in the same boat as you for oban so I will get to it at some point if it doesn't get done earlier.

@danschultzer
Copy link

danschultzer commented Dec 28, 2024

I've a lot of the 1.27 spec work for Oban done in #436 but there's more to it, and requires deeper review. I've asked some questions there and laid out all the changes.

I'm not sure if it's necessary to hold up fixing dependency conflicts as it seems there's backwards compability in 1.27 for 0.2? You'll just get deprecated warnings.

@bryannaegele
Copy link
Collaborator

We made the decision that updating to 1.27 will require breaking changes and just publishing updated versions that spew warnings isn't sufficient so let's just get it done with. The suggestion is something we evaluated and decided against at the time both for the volume of warnings that would be emitted and users seeing 1.27 in the dependencies but nothing matches the spec.

I understand it is frustrating.

@danschultzer
Copy link

danschultzer commented Dec 28, 2024

Ah ok. When I get a moment I'll go over #436 again to see what's missing. My goal was to only touch the attributes that were incorrect and fixing the span name to not make the PR too heavy. IIRC there are other new attributes that probably can be added, and we'll also need to get rid of the oban specific attributes, along with adding creation context.

@davidjulien
Copy link

Thank you for your explanations!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants