Skip to content

Comments

Fix versioned metrics#2316

Merged
pcapriotti merged 2 commits intodevelopfrom
pcapriotti/fix-versioned-metrics
Apr 27, 2022
Merged

Fix versioned metrics#2316
pcapriotti merged 2 commits intodevelopfrom
pcapriotti/fix-versioned-metrics

Conversation

@pcapriotti
Copy link
Contributor

This makes sure that every other middleware sees the rewritten (unversioned) path. In particular, the prometheus middleware will now only see paths it knows about, which prevents it from reporting "N/A" as the path.

Checklist

  • The PR Title explains the impact of the change.
  • The PR description provides context as to why the change should occur and what the code contributes to that effect. This could also be a link to a JIRA ticket or a Github issue, if there is one.
  • changelog.d contains the following bits of information (details):
    • A file with the changelog entry in one or more suitable sub-sections. The sub-sections are marked by directories inside changelog.d.

@pcapriotti pcapriotti temporarily deployed to cachix April 25, 2022 14:31 Inactive
@smatting
Copy link
Contributor

I hope that another side-effect of building without optimization (sorry I forgot this) is that cabal can use use caches from the ubuntu20-builder builder now, which also builds without optimization.

@jschaul
Copy link
Member

jschaul commented Apr 26, 2022

I hope that another side-effect of building without optimization (sorry I forgot this) is that cabal can use use caches from the ubuntu20-builder builder now, which also builds without optimization.

Don't we need to build images that are production ready, with optimizations? Those charts/images we release should have optimizations enabled, no?
For CI I agree it doesn't matter, but perhaps in some refactoring this was turned off at some point?

@pcapriotti pcapriotti force-pushed the pcapriotti/fix-versioned-metrics branch from 571dbb4 to 9f9ebbf Compare April 26, 2022 09:06
@pcapriotti pcapriotti temporarily deployed to cachix April 26, 2022 09:06 Inactive
@smatting
Copy link
Contributor

I hope that another side-effect of building without optimization (sorry I forgot this) is that cabal can use use caches from the ubuntu20-builder builder now, which also builds without optimization.

Don't we need to build images that are production ready, with optimizations? Those charts/images we release should have optimizations enabled, no? For CI I agree it doesn't matter, but perhaps in some refactoring this was turned off at some point?

Sorry, my comment was meant for another PR: https://github.com/zinfra/cailleach/pull/1027

Yes, production is build with optimization.

@pcapriotti pcapriotti force-pushed the pcapriotti/fix-versioned-metrics branch from 9f9ebbf to 52d3ebb Compare April 26, 2022 09:54
@pcapriotti pcapriotti temporarily deployed to cachix April 26, 2022 09:54 Inactive
This makes sure that every other middleware sees the rewritten
(unversioned) path. In particular, the prometheus middleware will now
only see paths it knows about, which prevents it from reporting "N/A" as
the path.
@pcapriotti pcapriotti force-pushed the pcapriotti/fix-versioned-metrics branch from 52d3ebb to 826f440 Compare April 27, 2022 06:29
@pcapriotti pcapriotti temporarily deployed to cachix April 27, 2022 06:29 Inactive
@pcapriotti pcapriotti merged commit 5ffb6cc into develop Apr 27, 2022
@pcapriotti pcapriotti deleted the pcapriotti/fix-versioned-metrics branch April 27, 2022 10:09
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