fix(ops): group OpenTelemetry module upgrades and sync semconv imports in Go files - #2015
fix(ops): group OpenTelemetry module upgrades and sync semconv imports in Go files#2015bwplotka wants to merge 1 commit into
Conversation
| # OpenTelemetry modules share versions and schema URLs across packages (e.g. otel, otel/sdk, otel/trace, otel/metric). | ||
| # Upgrade all otel modules present in go.mod together to avoid conflicting schema URL errors. |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the vulnerability fixing logic in ops/gmpctl/lib.sh to use go get for updating modules instead of direct sed replacements in go.mod. It also introduces special handling for OpenTelemetry modules to upgrade them simultaneously and align semconv imports to prevent conflicting Schema URL errors. The review feedback highlights potential script crashes due to the use of grep in pipelines under set -o pipefail and set -o errexit when no matches are found, and suggests using awk to safely handle these cases.
| echo "🔄 Updating OpenTelemetry modules simultaneously:${otel_args}..." | ||
| go get ${otel_args} | ||
|
|
||
| # OpenTelemetry SDK resource detectors (e.g. WithProcessRuntimeDescription, WithTelemetrySDK) |
There was a problem hiding this comment.
I am assuming we can equally in our fork remove the explicit schema at this point? 🤔
There was a problem hiding this comment.
Ok, we can delete the explicit scheme - confirmed with @dashpole https://github.com/GoogleCloudPlatform/prometheus/blob/a24099b1f7e42d86bae4e911d245fdcf932f7cff/tracing/tracing.go#L33
6763f5e to
6bca81e
Compare
…s in Go files - Uses `go list -m all` to include direct and indirect `go.opentelemetry.io/otel*` modules (such as `otel/trace`, `otel/sdk`, `otel/metric`) in simultaneous upgrades. - Dynamically queries the exact `semconv` package version imported by `go.opentelemetry.io/otel/sdk/resource` and updates hardcoded `semconv` imports across Go files to match. Signed-off-by: bwplotka <bwplotka@google.com>
6bca81e to
c5c876a
Compare
|
|
||
| # Read the vulnerability file line by line. | ||
| # The `|| [[ -n "$line" ]]` part handles the case where the last line doesn't have a newline. | ||
| pushd "${dir}" |
There was a problem hiding this comment.
Moving pushd before the while loop means relative vuln_file paths will fail at done <"${vuln_file}" since the working directory has changed. Resolving vuln_file to an absolute path before entering the loop ensures redirection always succeeds:
| pushd "${dir}" | |
| if [[ "${vuln_file}" != /* ]]; then | |
| vuln_file="$(pwd)/${vuln_file}" | |
| fi | |
| pushd "${dir}" |
| if [[ "${mod_path}" == go.opentelemetry.io/otel* ]]; then | ||
| # OpenTelemetry core API/SDK modules share versions and schema URLs across packages (e.g. otel, otel/sdk, otel/trace, otel/metric). | ||
| # Upgrade core otel modules present in the module graph together to avoid conflicting schema URL errors. | ||
| otel_mods=$(go list -m all 2>/dev/null | awk '{print $1}' | grep -E '^go\.opentelemetry\.io/otel($|/sdk$|/trace$|/metric$|/sdk/metric$|/log$|/sdk/log$)') |
There was a problem hiding this comment.
If no matching core OpenTelemetry modules are found in the module graph, grep will exit with status 1 and terminate the script due to set -o pipefail. Appending || true prevents this crash when zero matches occur:
| otel_mods=$(go list -m all 2>/dev/null | awk '{print $1}' | grep -E '^go\.opentelemetry\.io/otel($|/sdk$|/trace$|/metric$|/sdk/metric$|/log$|/sdk/log$)') | |
| otel_mods=$(go list -m all 2>/dev/null | awk '{print $1}' | grep -E '^go\.opentelemetry\.io/otel($|/sdk$|/trace$|/metric$|/sdk/metric$|/log$|/sdk/log$)' || true) |
| fi | ||
| done | ||
| echo "🔄 Updating OpenTelemetry modules simultaneously:${otel_args}..." | ||
| go get ${otel_args} |
There was a problem hiding this comment.
If none of the candidate modules pass the go list check, otel_args will remain empty and executing parameterless go get will fail with exit code 1 in modern Go module mode. Guarding the invocation avoids terminating the script when no packages match:
| go get ${otel_args} | |
| if [[ -n "${otel_args// /}" ]]; then | |
| go get ${otel_args} | |
| fi |
2 participants
Description
This PR fixes vulnerability updates for OpenTelemetry modules in
gmpctl:go list -m allto discover all direct and indirectgo.opentelemetry.io/otel*modules (such asotel/trace,otel/sdk,otel/metric, etc.) and upgrades them simultaneously to avoid version drift and partial module upgrades.semconvImport Synchronization: Dynamically queries the exactsemconvpackage version imported bygo.opentelemetry.io/otel/sdk/resourceand updates hardcodedsemconvimports in.gofiles. This preventsconflicting Schema URLerrors during tracer provider initialization (e.g.failed to install a new tracer provider: error detecting resource: conflicting Schema URL: https://opentelemetry.io/schemas/1.40.0 and https://opentelemetry.io/schemas/1.41.0).Signed-off-by: bwplotka bwplotka@google.com