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

fix version update script #5758

Closed
wants to merge 4 commits into from

Conversation

zeitlinger
Copy link
Member

The version script expects version updates to be grouped - which they were not

Here's run where a PR has updated only one of the files - and the second results it this:

https://github.com/open-telemetry/opentelemetry.io/actions/runs/12276199179/job/34252893445

> ./scripts/auto-update/version-in-file.sh opentelemetry-java-instrumentation instrumentation content/en/docs/zero-code/java/_index.md
HEAD is now at ceecd0f Auto-update registry versions (ba24a4546bf0ce12564c9d0ad7213f784242773e) (#5755)
REPO:            opentelemetry-java-instrumentation
LATEST VERSION:  v2.10.0
SEARCHING for:   '^ *instrumentation:' in content/en/docs/zero-code/java/_index.md
CURRENT VERSION:     instrumentation: 2.6.0
+ sed -i.bak -e 's/\(^ *instrumentation:\) .*/\1 2.10.0/' content/en/docs/zero-code/java/_index.md

Version update necessary:
diff --git a/content/en/docs/zero-code/java/_index.md b/content/en/docs/zero-code/java/_index.md
index f6723a6..2f594fd 100[64](https://github.com/open-telemetry/opentelemetry.io/actions/runs/12276199179/job/34252893445#step:4:65)4
--- a/content/en/docs/zero-code/java/_index.md
+++ b/content/en/docs/zero-code/java/_index.md
@@ -6,7 +6,7 @@ aliases:
   - /docs/languages/java/automatic_instrumentation
 cascade:
   vers:
-    instrumentation: 2.6.0
+    instrumentation: 2.10.0
     otel: 1.40.0
 ---
 

PR(s) already exist for 'Update opentelemetry-java-instrumentation version to v2.10.0'

@zeitlinger zeitlinger requested a review from a team as a code owner December 11, 2024 13:40
@zeitlinger zeitlinger self-assigned this Dec 11, 2024
@opentelemetrybot opentelemetrybot requested review from a team December 11, 2024 13:41
@opentelemetrybot opentelemetrybot requested review from TylerHelmuth and removed request for a team December 11, 2024 13:41
@zeitlinger
Copy link
Member Author

can't see why the build is failing

@chalin or @svrnm ?

Copy link
Contributor

@chalin chalin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zeitlinger, thanks for surfacing the issue and proposing a fix! 🙌🏻

I've created the following issue to track discussion about the source of the problem and followup fixes (like this PR -- we'll likely need to do more):

Regarding the build failures that we can see from https://github.com/open-telemetry/opentelemetry.io/actions/runs/12278556636/job/34260597540?pr=5758:

Run scripts/check-build-log.sh
WARNINGs or ERRORs found in build log:
ERROR Param "vers.collector" not found: "/home/runner/work/opentelemetry.io/opentelemetry.io/content/en/docs/collector/installation.md:19:10"
ERROR Param "vers.collector" not found: "/home/runner/work/opentelemetry.io/opentelemetry.io/content/en/docs/collector/installation.md:81:92"
ERROR Param "vers.collector" not found: "/home/runner/work/opentelemetry.io/opentelemetry.io/content/en/docs/collector/installation.md:1:1"
Error: error building site: logged 3 error(s)

This is actually due (I think) to:

Let me investigate further and get back to you.

@chalin
Copy link
Contributor

chalin commented Dec 11, 2024

Actually, it's not even because of #5761. I'm stumped as to why those errors are being reported. The same font-matter params work in another section, but not under the Collector section. I'll continue to investigate as soon as I can, and will keep y'all posted. Converting to draft in the meantime.

@chalin
Copy link
Contributor

chalin commented Dec 12, 2024

@zeitlinger - as I mention in #5769, which I proposed as an alternative to this PR, I just can't figure our the source of the failures in this PR. There's no reason for the cascaded fields not to be picked up. I'd have to try to create a small test case, which I don't have time for atm. PTAL #5769 and let me/us know if this works for you.

@zeitlinger zeitlinger closed this Dec 12, 2024
@zeitlinger zeitlinger deleted the version-update branch December 12, 2024 09:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants