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

Prometheus: Fix quote stripping in parser #87675

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ire4ever1190
Copy link

Currently only double quotes are stripped from the end, while other quotes are left in. And quotes are stripped even when part of the value. This PR makes it so all quotes used in PromQL are stripped from beginning/end and also not removed from the value

Before

image
Extra quotes are added for instance, and the escaped quote is removed from job

After

image

Very minor fix but its be subtly annoying me 😅

@ire4ever1190 ire4ever1190 requested a review from a team as a code owner May 11, 2024 11:49
@CLAassistant
Copy link

CLAassistant commented May 11, 2024

CLA assistant check
All committers have signed the CLA.

@grafana-pr-automation grafana-pr-automation bot added area/frontend pr/external This PR is from external contributor labels May 11, 2024
@itsmylife
Copy link
Contributor

itsmylife commented May 13, 2024

@ire4ever1190 Thank for the contribution! It looks ok to me but I'd like to get second opinion from @grafana/observability-metrics too.
@grafana/observability-metrics Do you see any downsides in this approach?

@itsmylife itsmylife modified the milestones: 11.0.x, 11.1.x May 13, 2024
@itsmylife itsmylife added datasource/Prometheus no-backport Skip backport of PR no-changelog Skip including change in changelog/release notes labels May 13, 2024
@ire4ever1190
Copy link
Author

Failure looks unrelated?

Error: Input required and not supplied: app_id

@itsmylife
Copy link
Contributor

itsmylife commented May 15, 2024

@ire4ever1190 Yes that's unrelated and won't affect the build for now.

@itsmylife
Copy link
Contributor

@ire4ever1190 Could you please merge the latest main branch on your branch. I guess CI will be happy that way.

Currently only double quotes are stripped from the end, while single quotes are left. Moreover, double quotes are stripped even when part of the value
@ire4ever1190
Copy link
Author

Rebased with main

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/frontend datasource/Prometheus no-backport Skip backport of PR no-changelog Skip including change in changelog/release notes pr/external This PR is from external contributor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants