Skip to content

fix(discovery): aws discovery test fix#17527

Merged
aknuds1 merged 2 commits intoprometheus:mainfrom
wbollock:fix/aws_discover_test
Nov 16, 2025
Merged

fix(discovery): aws discovery test fix#17527
aknuds1 merged 2 commits intoprometheus:mainfrom
wbollock:fix/aws_discover_test

Conversation

@wbollock
Copy link
Contributor

Fixes a problem introduced after the merge of this #17138

PR didn't take into account another merged PR!

discovery/aws/aws.go:218:54: too many arguments in call to NewEC2Discovery
    have (*EC2SDConfig, *slog.Logger, *ec2Metrics)
    want (*EC2SDConfig, discovery.DiscovererOptions)
discovery/aws/aws.go:222:66: too many arguments in call to NewLightsailDiscovery
    have (*LightsailSDConfig, *slog.Logger, *lightsailMetrics)
    want (*LightsailSDConfig, discovery.DiscovererOptions)

Does this PR introduce a user-facing change?

NONE

@wbollock
Copy link
Contributor Author

first commit is superseded by #17526

@aknuds1
Copy link
Contributor

aknuds1 commented Nov 13, 2025

Can you rebase on main please? Due to overlap with #17526.

@aknuds1
Copy link
Contributor

aknuds1 commented Nov 13, 2025

Never mind, I could diff locally.

Copy link
Contributor

@aknuds1 aknuds1 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

Fixes a problem introduced after the merge of this prometheus#17138

PR didn't take into account another merged PR!

```
discovery/aws/aws.go:218:54: too many arguments in call to NewEC2Discovery
    have (*EC2SDConfig, *slog.Logger, *ec2Metrics)
    want (*EC2SDConfig, discovery.DiscovererOptions)
discovery/aws/aws.go:222:66: too many arguments in call to NewLightsailDiscovery
    have (*LightsailSDConfig, *slog.Logger, *lightsailMetrics)
    want (*LightsailSDConfig, discovery.DiscovererOptions)
```

Signed-off-by: Will Bollock <wbollock@linode.com>
ECS was a new service discovery tool added after this PR was merged: prometheus#17138

Aligns the style of passing a single "opts" to it like almost all the other
service discovery engines now use

Signed-off-by: Will Bollock <wbollock@linode.com>
@wbollock wbollock force-pushed the fix/aws_discover_test branch from 93a3c63 to e5c93cd Compare November 13, 2025 16:39
@wbollock
Copy link
Contributor Author

rebased anyways! thanks

@aknuds1 aknuds1 enabled auto-merge (squash) November 13, 2025 16:56
@aknuds1
Copy link
Contributor

aknuds1 commented Nov 16, 2025

Can you try rebasing on main please? The consistently flaking TestRemoteWrite_ReshardingWithoutDeadlock is skipped there.

@aknuds1 aknuds1 merged commit 4aa8941 into prometheus:main Nov 16, 2025
181 of 190 checks passed
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

Successfully merging this pull request may close these issues.

2 participants