[WOR-1560] Upgrade to TCL 1.1.1 for MDC-aware Stairway #418
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
MdcHook is greatly reduced and renamed to StairwayLoggingHook: it now only logs at notable state transitions.
I learned that Spring Boot Dependency Manager pulls in opentelemetry-bom as a dependency.
For our Spring Boot version 3.1.2, opentelemetry-bom has version 1.25.0.
Using the BOM (bill of materials) to manage OTEL dependencies is preferred to managing them individually: the BOM makes sure that they remain compatible with one another.
The only OTEL dependencies that BPM needs to define directly are:
1. opentelemetry-api so that we can construct OpenTelemetry objects (versioned by the BOM, which is itself versioned by Spring Boot dependency manager)
2. opentelemetry-instrumentation-annotations so that we can use the @WithSpan annotation
terra-common-lib defines the OTEL deps it needs, they are available as runtime dependencies so we don't need to redefine them.
But it does not use the BOM to version them, or Spring Boot dependency manager. It versions them directly at 1.34.1.
I found when removing the OTEL deps that BPM didn't need, I also needed to instruct our Spring Boot dependency manager to pin opentelemetry-bom at a higher version compatible with TCL's OTEL dependencies.
TCL 1.1.1 introduces a common StairwayLoggingHook for reuse, so I deleted BPM's. Spring Boot 3.2.3 addresses a number of reported vulnerabilities.
okotsopoulos
changed the title
Given the extent of dependency modifications I made, I appreciated having a way to check locally that I was both addressing vulnerabilities, and not pulling in any new ones. Some other teams also run SourceClear as part of their CI build process, but I am not making that change at this time.
okotsopoulos
requested review from
blakery,
sergiygetlin and
cahrens
and removed request for
blakery
|
cahrens
reviewed
Mar 27, 2024
| @@ -76,5 +76,23 @@ Automatically fix linting issues: | |||
| ./gradlew generateSwaggerCode | |||
| ``` | |||
|
|
|||
| ### Running SourceClear locally | |||
There was a problem hiding this comment.
Thank you for documenting this!!
There was a problem hiding this comment.
My pleasure! I will have a follow-on to get this in LZS too, and will get it in WSM.
sergiygetlin
approved these changes
Mar 27, 2024
| @@ -5,6 +5,7 @@ plugins { | |||
|
|
|||
| id 'com.diffplug.spotless' | |||
| id 'org.hidetake.swagger.generator' | |||
| id 'com.srcclr.gradle' | |||
There was a problem hiding this comment.
Is this tool used across different project in Terra? Is it something new or BPM haven't used it yet?
There was a problem hiding this comment.
It is used in a number of Terra projects, but it's wired in inconsistently.
Here are a few examples: https://github.com/search?q=org%3ADataBiosphere+com.srcclr.gradle%3Acom.srcclr.gradle.gradle.plugin&type=code
Here's an example of a repo that has Gradle's srcclr plug-in enabled, and also runs the scan as part of their builds:
https://github.com/search?q=repo%3ADataBiosphere%2Fjava-pfb%20srcclr&type=code
LZS actually pulls in the plug-in, but doesn't currently use it anywhere.
cahrens
approved these changes
Mar 27, 2024
okotsopoulos
added a commit
that referenced
this pull request
Mar 28, 2024
This reverts commit 530a866. Reason: TCL's upgrade to k8s client 20.0.1 broke its KubePodListener, which could affect the recovery of Stairway flights.
okotsopoulos
added a commit
that referenced
this pull request
Mar 28, 2024
okotsopoulos
added a commit
that referenced
this pull request
Apr 1, 2024
* Revert "Revert "[WOR-1560] Upgrade to TCL 1.1.1 for MDC-aware Stairway (#418)" (#425)" This reverts commit 9220e78. Reason: we are going to upgrade TCL to version k8s client at 20.0.1-legacy rather than 20.0.1. The rest of the commit that we originally reverted we'd like to apply. * Upgrade TCL to 1.1.4 This upgrade versions our transitive dependency on k8s client at 20.0.1-legacy -- the latest legacy version which moves past a number of vulnerabilities, but does not introduce breaking changes. * Do not useMavenLocal by default
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
https://broadworkbench.atlassian.net/browse/WOR-1560
Upgraded
terra-common-libfrom 0.1.6 -> 1.1.1 mainly for the benefit of Stairway 1.0.0's native MDC-awareness.As a result, MdcHook is removed in favor of TCL's common StairwayLoggingHook, which only logs at notable state transitions.
BPM now also gets more context than it previously had. Previously, it only logged the context of the calling thread for logs emitted during the execution of each step. Now, it…
This matches what's been live in WSM for awhile.
Dependabot for Spring Boot upgrades
One of the most impactful things we can do to stay on top of vulnerable dependencies is to keep Spring Boot up to date. We use Spring's dependency manager and pull in many of its starter packages as dependencies, so as Spring Boot addresses vulnerabilities on its ends we'd like to get them for free, rather than be tempted to pin versions solely for the purpose of mitigating a vulnerability.
So, I've adjusted our Dependabot settings to stop ignoring Spring Boot updates.
Running SourceClear scans locally
I added Gradle's srcclr plugin and instructions for local scans, following examples set by other repos. Given the extent of dependency modifications in this PR, I appreciated having a way to check locally that I was both addressing vulnerabilities, and not pulling in any new ones.
Some other teams also run SourceClear as part of their CI build process, but I am not making that change at this time.
Other dependency changes
I upgraded Spring Boot to latest, 3.2.3, and cleaned up dependencies and pins no longer needed. This upgrade will address a number of vulnerabilities flagged by Veracode, confirmed by running SourceClear locally off of this feature branch.
OpenTelemetry direct dependencies could be drastically reduced, but we do have to make sure that Spring Boot dependency manager is bringing in a version of
opentelemetry-bomcompatible with what TCL is using.