Skip to content

Conversation

@vishalegbert-ttd
Copy link
Contributor

No description provided.

@aulme
Copy link
Contributor

aulme commented Nov 22, 2024

Thanks for fixing, happy to merge as is.

I think it would still be better if we scraped routes from the router as this approach won't work if we use nested routers and someone may forget adding their new paths to the enum resulting in no obvious failure but no metrics. Don't know if you've tried looking at this, eg the way it's described here.

@vishalegbert-ttd vishalegbert-ttd merged commit b99a748 into main Nov 25, 2024
3 of 4 checks passed
@vishalegbert-ttd vishalegbert-ttd deleted the vse-UID2-4504-fix-unbounded-path-label branch November 25, 2024 01:32
@vishalegbert-ttd
Copy link
Contributor Author

vishalegbert-ttd commented Nov 25, 2024

Thanks @aulme , yes I thought about it. But this is now an established pattern in Operator already: https://github.com/IABTechLab/uid2-operator/blob/main/src/main/java/com/uid2/operator/vertx/Endpoints.java and the routes object isn't exposed to where the metric filtering rule is defined

There's a future ticket to improve the test coverage of this metric filtering, we might try to use the registered routes during that ticket

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.

3 participants