Skip to content

fix(renovate): improve lockfile maintenance and tidy config #2546

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.

Sign up for GitHub

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

Open
wpessers wants to merge 3 commits into
base: main
Choose a base branch
from
+5 −13
Open

fix(renovate): improve lockfile maintenance and tidy config #2546

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c5397c7
fix(renovate): remove gitIgnoredAuthors to avoid infinite loop
wpessers Aug 20, 2026
267ec66
feat(renovate): use gomodTidyAll to automatically tidy dependent modules
wpessers Aug 20, 2026
56d65e5
fix(renovate): only run lockfile maintenance for minor and patch vers…
wpessers Aug 20, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 5 additions & 13 deletions .github/renovate.json5
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,6 @@
'helpers:pinGitHubActionDigestsToSemver',
':separateMultipleMajorReleases',
],
gitIgnoredAuthors: [
"107717825+opentelemetrybot@users.noreply.github.com",
"107717825+opentelemetrybot[bot]@users.noreply.github.com",
"197425009+otelbot@users.noreply.github.com",
"197425009+otelbot[bot]@users.noreply.github.com",
"github-actions[bot]@users.noreply.github.com"
],
Comment on lines -8 to -14

@thompson-tomo thompson-tomo Aug 23, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think removing this is not necessary as it stops the rebase frequency but does so by putting the pr in an edited state blocking updates and then it potentially error if more updates become available. See below for an alternative.

The issue is renovate is seeing that it is out of date as a transitive has changed hence it triggers a force push to update those transitive.

@wpessers wpessers Aug 24, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I really don't think those force pushes are driven by transitive changes. I verified this looking specifically into PR #2542 -> 200 force-pushes contain just 17 distinct trees. 182 consecutive pushes are identical, e.g.1f1ce2e2 and 342e6846 this can be checked via the gh api or using the cli:
gh api repos/open-telemetry/opentelemetry-lambda/git/commits/<commit hash> --jq .tree.sha.

All 200 overwrote a tidy commit by otelbot through the gh workflow we have for that, with un-tidy content. The "edited state" after removal is intentional and I was even more confident in this change when I cross-referenced with the collector and contrib repos, and noticed it's exactly how they run the same tidy workflow (they don't set gitIgnoredAuthors).

@thompson-tomo thompson-tomo Aug 25, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Sure some of them could be the same but some are transitive. If the concern is qty, then what about setting commitLimit to 1 on the lockfile, this way we maintain the ci running. With removing this we block renovate from updating the collector pr's etc.

prHourlyLimit: 15,
prConcurrentLimit: 15,
ignorePaths: [
Expand All @@ -33,7 +26,7 @@
'golang',
],
postUpdateOptions: [
'gomodTidy',

@thompson-tomo thompson-tomo Aug 23, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looking at the logs, https://developer.mend.io/github/open-telemetry/opentelemetry-lambda/-/job/01a00d17-448a-7bb4-94dc-309930aa60b6 it looks like part of the issue is when renovate runs goModTidy it is failing with an error. What i can't tell is why it fails when run by renovate but ok as a ci job.

The only thing it could be is that the workflow is not checking exit Code. We could add the gomodTidyE option to renovate in addition to goModTidyAll and see if that decreases lines changed by the workflow.

@wpessers wpessers Aug 24, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I'm not sure this is related, all those failures were for a completely different renovate PR. They're for the one that updates the collector-other deps.

As stated above also, on the lockfile PRs we know for sure that gomodTidy is being skipped, so this won't affect those lockfile PRs AFAICT.

@thompson-tomo thompson-tomo Aug 25, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actually it is related as if we remove the ignored authors, pr's such as that will end up in the edited/errored state and blocked from adding additional updates.

'gomodTidyAll',
],
semanticCommitType: 'build',
schedule: [
Expand All @@ -53,7 +46,7 @@
'golang',
],
postUpdateOptions: [
'gomodTidy',
'gomodTidyAll',
],
semanticCommitType: 'build',
schedule: [
Expand Down Expand Up @@ -176,17 +169,16 @@
{
groupName: "Lock file maintenance golang",

@thompson-tomo thompson-tomo Aug 23, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I would suggest adding rebasewhen to reduce when the rebase occurs by setting it to behind-base-branch

@wpessers wpessers Aug 24, 2026 •
edited
Loading

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

rebaseWhen doesn't affect these pushes. What we're seeing here aren't rebases, renovate says so itself in the logs. In the Mend log (https://developer.mend.io/github/open-telemetry/opentelemetry-lambda/-/job/733c6999-a1c0-47f6-8e53-ac6ba49c67ee, log level Debug, branch renovate/lock-file-maintenance-golang) the run logs, in order:

DEBUG: Branch + PR exists but is not scheduled -- will update if necessary (branch="renovate/lock-file-maintenance-golang")
DEBUG: Converting rebaseWhen=auto to rebaseWhen=conflicted because no rule for behind-base-branch applies (branch="renovate/lock-file-maintenance-golang")
DEBUG: Branch already exists (branch="renovate/lock-file-maintenance-golang")
DEBUG: Skipping behind base branch check due to rebaseWhen=conflicted (branch="renovate/lock-file-maintenance-golang")
...
DEBUG: branch.isConflicted(): false (branch="renovate/lock-file-maintenance-golang")
DEBUG: Branch does not need rebasing (branch="renovate/lock-file-maintenance-golang")

And then later on it pushes:

DEBUG: 14 file(s) to commit (branch="renovate/lock-file-maintenance-golang")
...
INFO: Branch updated (branch="renovate/lock-file-maintenance-golang")
{
  "commitSha": "70a79d988848cb9a10b333cf3fcbd5a4e4c0fb23"
}

The pushes we see come from the "content regeneration" path, which branch.isModified() controls, and gitIgnoredAuthors makes that return false (you can see branch.isModified() = false in the same log), which is why the tidy commit gets overwritten. So changing the rebaseWhen config doesn't really impact what is going on here.

@thompson-tomo thompson-tomo Aug 25, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't think the cause is the ignoredAuthors, all that is doing is enabling the pr to avoid an edited/error state. It is more likely renovate updates a dependency to a version which is not imported & then go mod tidy removes it. Hence the fix might be to actually get renovate to do the go tidy as if it adds it then go tidys it away then there will be nothing changed & should avoid force push.

@wpessers wpessers Aug 25, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I'm not saying it's the cause. I am saying it could be some sort of safeguard though. And looking at collector and collector-contrib made me wonder if that's the same reason they don't specify the ignored authors either.

@thompson-tomo thompson-tomo Aug 25, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

They differ as they don't update indirect. The safeguard of removing the gitIgnoreAuthors is the same as auto adding the stop updating label.

matchDepTypes: ["indirect", "replace"],
matchUpdateTypes: ["minor", "patch"],
matchCategories: [
'golang',
],
enabled: true,
separateMajorMinor: false,
separateMultipleMajor: false,
Comment on lines +172 to -184

@thompson-tomo thompson-tomo Aug 23, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I am not following

Only update minor and patch versions in the lockfile maintenance, "updating" majors of indirect deps is a no-op but it has the side-effect of gomodTidy not being run because renovate thinks the branch contains an update major version.

Is what you are thinking is that the presence of a major triggers gomodTidy to not be run?

I would be inclined to leave but instead adjust when the rebase is done as well as adjust the postupdateoptions.

@wpessers wpessers Aug 24, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yeah exactly, I'll refer to renovate's actual code here again for reference just like I did in our Slack conversation: https://github.com/renovatebot/renovate/blob/34c7bcb7a50cafe7c8fd7f94fc679d0325bb3ccc/lib/modules/manager/gomod/artifacts.ts#L273-L278

As you can see later on in that same file, actually each variant of gomodTidy we can run in renovate sits behind that check: https://github.com/renovatebot/renovate/blob/34c7bcb7a50cafe7c8fd7f94fc679d0325bb3ccc/lib/modules/manager/gomod/artifacts.ts#L291-L302

So "keep majors and adjust postupdateoptions" won't make any difference here. There is no possible way to make renovate run any of the gomodTidy* variants as long as it's a branch classified by renovate as one that contains a major update, regardless of whether or not it actually does.

That last part is especially important and it took me a while to understand myself, but in go new major versions are actually new module paths. So for example an offending module I observed in one of the logs was golang-lru, it has a v1 github.com/hashicorp/golang-lru and v2 github.com/hashicorp/golang-lru/v2. The latter is an entirely different module, so there is no valid way for us (or renovate) to bump our existing requires. Changing it to github.com/hashicorp/golang-lru v2.0.0 is just invalid, since v2 versions must live at the /v2 path. Adding a brand new require line for the /v2 path doesn't survive either: nothing in the module graph will be importing that path (the code importing golang-lru lives inside our upstream deps, not in our own repo), so go mod tidy would immediately drop that line as unused. You can also verify that none of our past force pushes (or any commit really) on those renovate lockfile maintenance PRs have ever contained major version changes. For an official source, see: https://go.dev/ref/mod#major-version-suffixes

You can see the tidy skip happen in job https://developer.mend.io/github/open-telemetry/opentelemetry-lambda/-/job/733c6999-a1c0-47f6-8e53-ac6ba49c67ee where you will see lines like:

DEBUG: gomod: major update for github.com/hashicorp/golang-lru (branch="renovate/lock-file-maintenance-golang")

and a similar one for another dep github.com/cenkalti/backoff/v5. These logs are followed later on by a log indicating that the tidy command has been skipped, this happens for each module updated in that branch:

DEBUG: go mod tidy command skipped (branch="renovate/lock-file-maintenance-golang")

All this to simply say, we lose nothing by excluding majors from the indirect rule. As explained above, those indirect major "updates" can't ever land in a go.mod file, so excluding them removes nothing. What we gain here is that the weekly lockfile branch stops being classified as containing major updates, meaning renovate's own gomodTidy* should finally run on these PRs instead of being skipped. The removal of separateMajorMinor: false / separateMultipleMajor: false in this same commit is just cleanup. Those two settings only existed include majors into this branch in a specific way, and with the majors excluded there is no more need for that config.

@thompson-tomo thompson-tomo Aug 25, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for sharing the code, i think it points out the actual issue:

const mustSkipGoModTidy =
      !config.postUpdateOptions?.includes('gomodUpdateImportPaths') &&
      config.updateType === 'major';


const isGoModTidyAllRequired =
      config.postUpdateOptions?.includes('gomodTidyAll') === true;

const isGoModTidyRequired =
      !mustSkipGoModTidy &&
      (config.postUpdateOptions?.includes('gomodTidy') === true ||
        config.postUpdateOptions?.includes('gomodTidy1.17') === true ||
        config.postUpdateOptions?.includes('gomodTidyE') === true ||
        isGoModTidyAllRequired ||
        (config.updateType === 'major' && isImportPathUpdateRequired));

Based on that code the likely cause of our issue is that mustSkipGoModTidy is returning true hence it is not running. To address this we should set gomodUpdateImportPaths, that way it should run without needing to remove major updates.

So "keep majors and adjust postupdateoptions" won't make any difference here. There is no possible way to make renovate run any of the gomodTidy* variants as long as it's a branch classified by renovate as one that contains a major update, regardless of whether or not it actually does.

Based on the above code, if we set both gomodUpdateImportPaths & a gomodTidy* option it should update major given the usage of ||.

You can also verify that none of our past force pushes (or any commit really) on those renovate lockfile maintenance PRs have ever contained major version changes.

Not the case, If you take a look at https://github.com/open-telemetry/opentelemetry-lambda/pull/2495/changes#diff-84e81f625eb3580b5a4892a5b5ee1d4c441707ce0655c99621c1cbf3f1d80fb4L41 there is 3 examples of major updates being successful through renovate.

schedule: [
'before 8am on Wednesday',
],
postUpdateOptions: [
'gomodTidy',
'gomodTidyAll',
],
}
],
Expand All @@ -199,7 +191,7 @@
],
},
postUpdateOptions: [
'gomodTidy',
'gomodTidyAll',
],
semanticCommits: 'enabled',
semanticCommitType: 'build',
Expand Down
Loading