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: update prometheus.rules.json with new schema #997

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

monteiro-renato
Copy link
Contributor

@monteiro-renato monteiro-renato requested a review from a team as a code owner October 16, 2024 20:39
@vishiy
Copy link
Contributor

vishiy commented Oct 16, 2024

Thanks for your PR @monteiro-renato .

@vishiy
Copy link
Contributor

vishiy commented Oct 16, 2024

@moshemal - can one of you validate this data type addition for expr and approve, after which we can merge ?

@monteiro-renato
Copy link
Contributor Author

@microsoft-github-policy-service agree

Copy link

This PR is stale because it has been open 7 days with no activity. Remove stale label or comment or this will be closed in 5 days.

@monteiro-renato
Copy link
Contributor Author

Still interested but awaiting feedback from codeowners.

@vishiy
Copy link
Contributor

vishiy commented Nov 7, 2024

/azp run

Copy link

No pipelines are associated with this pull request.

@monteiro-renato
Copy link
Contributor Author

I noticed a new PR will fix the tests > #1008 so I reverted my changes and this PR will only have the changes to the schema.

Copy link

This PR is stale because it has been open 7 days with no activity. Remove stale label or comment or this will be closed in 5 days.

@monteiro-renato
Copy link
Contributor Author

Still interested but awaiting feedback from codeowners.

@vishiy
Copy link
Contributor

vishiy commented Nov 21, 2024

@moshemal - please check and review the schema changes and add Noam if needed...

@vishiy
Copy link
Contributor

vishiy commented Nov 21, 2024

/azp run

Copy link

No pipelines are associated with this pull request.

@monteiro-renato
Copy link
Contributor Author

Hey 👋
I've added both changes I suggested to my own fork last week and I saw no issues deploying the ARM templates: https://github.com/monteiro-renato/prometheus-collector/tree/with-piped-input.

Here's an example of what I was trying to deploy initially and failing due to the schema only accepting strings in the expr field:
bilde
Not sure how testing is done on your side but you can add a test where the expression field in the prometheus rule is a number instead of a string and see how it goes.

@monteiro-renato
Copy link
Contributor Author

Partial yaml config:

  groups:
  - interval: 60s
    name: traefik-availability-generic
    rules:
    - expr: "0.9990000000000001"
      labels:
        slo: traefik-availability
      record: pyrra_objective
    - expr: 2419200
      labels:
        slo: traefik-availability
      record: pyrra_window

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