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

a bom for spectator? #920

Open
dbyron-sf opened this issue Nov 4, 2021 · 6 comments
Open

a bom for spectator? #920

dbyron-sf opened this issue Nov 4, 2021 · 6 comments

Comments

@dbyron-sf
Copy link
Contributor

When using spectator in https://github.com/spinnaker/kork, I'd like to be able to decide what version of the aws sdk to use, potentially even if the one that spectator depends on is newer. Currently, kork defines it's spectator dependency here like:

dependencies {
  constraints {
    api("com.netflix.spectator:spectator-api:${versions.spectator}")
    api("com.netflix.spectator:spectator-ext-aws:${versions.spectator}")
    api("com.netflix.spectator:spectator-ext-gc:${versions.spectator}")
    api("com.netflix.spectator:spectator-ext-jvm:${versions.spectator}")
    api("com.netflix.spectator:spectator-nflx-plugin:${versions.spectator}")
    api("com.netflix.spectator:spectator-reg-atlas:${versions.spectator}")
    api("com.netflix.spectator:spectator-web-spring:${versions.spectator}")
    api("com.netflix.spectator:spectator-reg-micrometer:${versions.spectator}")
  }
}

If there were a spectator bom that kork could include via something like:

dependencies {
  api(platform("com.netflix.spectator:spectator-bom:${versions.spectator}")
}

I believe that would give more control to kork over which versions of transitive dependencies to bring in. Specifically for the aws java sdk case:

dependencies {
  api(platform("com.amazonaws:aws-java-sdk-bom:${versions.aws}"))
  api(platform("com.netflix.spectator:spectator-bom:${versions.spectator}")

gives precedence to the aws java sdk bom because it appears first.

It could easily be that my gradle knowledge is insufficient and there is a better or easier way to handle this. I would love a hand figuring out a solution, whether it means a spectator bom or not.

Thanks much for your help.

@brharrington
Copy link
Contributor

If I understand correctly, then your concern is that Kork started getting a newer version of the AWS SDK after updating Spectator. Is that correct? I haven't looked into BOMs all that much, so would have to research it a bit, but probably won't have time for a while.

@dbyron-sf
Copy link
Contributor Author

Turns out it's not the aws sdk that's causing grief, especially after Netflix/awsobjectmapper#108. It's jackson. Version 2.13.0 of jackson-module-kotlin brings in kotlin 1.5.x where 2.12.3 (which spinnaker currently uses bring in kotlin 1.4.x and making that leap appears to be a lot of work.

Definitely appreciate the help here. I took a crack at adding a bom to spectator, but haven't made much progress so far. In theory https://github.com/spinnaker/kork/blob/master/kork-bom/kork-bom.gradle and https://github.com/spinnaker/kork/blob/master/spinnaker-dependencies/spinnaker-dependencies.gradle are reasonable models to follow, but there's enough other stuff going on here that it didn't "just work."

@brharrington
Copy link
Contributor

Looking at the dependencies in the list above, the only one that would bring in jackson is spectator-web-spring. At first glance it only appears to be used in one place. It could probably be updated to be an implementation dependency instead of api, but not sure that would matter for what you are seeing. However, given it is not part of the api it could be shaded similar to what we do for spectator-reg-atlas, then from your perspective there wouldn't be a transitive jackson dependency.

@dbyron-sf
Copy link
Contributor Author

Shading jackson sounds lovely. Yes please! Yeah, switching to an implementation dependency sounds like a good idea, but probably wouldn't help overall ... since I think jackson would still end up as a runtime dependency.

brharrington added a commit to brharrington/spectator that referenced this issue Nov 6, 2021
This should help minimize issues with other uses that
might need to use older versions of jackson (e.g. Netflix#920).
brharrington added a commit that referenced this issue Nov 6, 2021
This should help minimize issues with other uses that
might need to use older versions of jackson (e.g. #920).
@brharrington
Copy link
Contributor

1.0.5 has been released and shades jackson for spectator-web-spring.

@dbyron-sf
Copy link
Contributor Author

That definitely helps. There's still a struggle with a mismatch between spectator-web-spring's spring dependencies (2.5.6 for boot and 5.3.12 for spring) and what spinnaker uses (2.2.13 for boot and 5.2.2 for spring). With some careful exclusions I think I can deal. Shading that much of spring doesn't feel like a great solution, though nothing else comes to mind.

dbyron-sf added a commit to dbyron-sf/kork that referenced this issue Nov 9, 2021
…s from api to implementation

since consumers of neither kork-stackdriver nor kork-web need it at compile time

Add a test to ensure that the metrics endpoint from spectator-web-spring is really available.

This paves the way to upgrade to spectator 1.0.5 where a newer version of the spring(boot)
dependencies that spectator-web-spring brings in cause problems.

See Netflix/spectator#920 for background
dbyron-sf added a commit to dbyron-sf/kork that referenced this issue Nov 9, 2021
- change scope of spectator-web-spring dependencies from api to implementation since
  consumers of neither kork-stackdriver nor kork-web need it at compile time, and exclude
  transitives to keep spectator-web-spring's spring(boot) dependencies from overriding
  what we've defined elsewhere.  Add a test to ensure that the metrics endpoint from
  spectator-web-spring is really available.

- exclude transitive dependencies of spectator-ext-aws in kork-aws for a similar reason --
  to keep spectator-ext-aws' version of the aws sdk (and the aws sdk's transitives) from
  giving us newer versions of the sdk, jackson and kotlin than consumers are ready for.

This paves the way to upgrade to spectator 1.0.5 where a newer version of the spring(boot)
dependencies that spectator-web-spring brings in cause problems.

See Netflix/spectator#920 for background.
dbyron-sf added a commit to spinnaker/kork that referenced this issue Nov 11, 2021
* chore(dependencies): clean up spectator dependencies

- change scope of spectator-web-spring dependencies from api to implementation since
  consumers of neither kork-stackdriver nor kork-web need it at compile time, and exclude
  transitives to keep spectator-web-spring's spring(boot) dependencies from overriding
  what we've defined elsewhere.  Add a test to ensure that the metrics endpoint from
  spectator-web-spring is really available.

- exclude transitive dependencies of spectator-ext-aws in kork-aws for a similar reason --
  to keep spectator-ext-aws' version of the aws sdk (and the aws sdk's transitives) from
  giving us newer versions of the sdk, jackson and kotlin than consumers are ready for.

This paves the way to upgrade to spectator 1.0.5 where a newer version of the spring(boot)
dependencies that spectator-web-spring brings in cause problems.

See Netflix/spectator#920 for background.

* chore(dependences): use version 1.0.6 of spectator to stay up to date
ylebedeva pushed a commit to ylebedeva/kork that referenced this issue May 3, 2022
* chore(dependencies): clean up spectator dependencies

- change scope of spectator-web-spring dependencies from api to implementation since
  consumers of neither kork-stackdriver nor kork-web need it at compile time, and exclude
  transitives to keep spectator-web-spring's spring(boot) dependencies from overriding
  what we've defined elsewhere.  Add a test to ensure that the metrics endpoint from
  spectator-web-spring is really available.

- exclude transitive dependencies of spectator-ext-aws in kork-aws for a similar reason --
  to keep spectator-ext-aws' version of the aws sdk (and the aws sdk's transitives) from
  giving us newer versions of the sdk, jackson and kotlin than consumers are ready for.

This paves the way to upgrade to spectator 1.0.5 where a newer version of the spring(boot)
dependencies that spectator-web-spring brings in cause problems.

See Netflix/spectator#920 for background.

* chore(dependences): use version 1.0.6 of spectator to stay up to date
richard-timpson pushed a commit to richard-timpson/kork that referenced this issue May 3, 2023
* chore(dependencies): change scope of spectator-web-spring dependencies from api to implementation

since consumers of neither kork-stackdriver nor kork-web need it at compile time

Add a test to ensure that the metrics endpoint from spectator-web-spring is really available.

This paves the way to upgrade to spectator 1.0.5 where a newer version of the spring(boot)
dependencies that spectator-web-spring brings in cause problems.

See Netflix/spectator#920 for background

@W-10103873

* chore(dependences): use version 1.0.5 of spectator to stay up to date

@W-10103873

* chore(dependencies): exclude transitive dependencies of spectator-ext-aws in kork-aws

to keep spectator-ext-aws' version of the aws sdk (and the aws sdk's transitives) from
giving us newer versions of the sdk, jackson and kotlin than consumers are ready for.

@W-10103873
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants