Skip to content

[query] return 0 if there was at least one valid number#2402

Draft
gediminasgu wants to merge 39 commits intomasterfrom
gg/only-nan-fix
Draft

[query] return 0 if there was at least one valid number#2402
gediminasgu wants to merge 39 commits intomasterfrom
gg/only-nan-fix

Conversation

@gediminasgu
Copy link
Copy Markdown
Collaborator

What this PR does / why we need it:
PromQL compatibility fix.
When running instant queries PromQL returns "0" in place of NaN if there was at least one valid number in a range. Otherwise, it returns "NaN". That is the fix for M3 query to behave in the same way.

Special notes for your reviewer:
I'll change base branch to master when depending PR will be merged.

Does this PR introduce a user-facing and/or backwards incompatible change?:

NONE

Does this PR require updating code package or user-facing documentation?:

NONE

Gediminas and others added 30 commits May 4, 2020 17:00
Allows to run queries like {tag="test"}. Previously only querying with metric name like random{tag="test"} was only supported.
…wo-metric-name

# Conflicts:
#	src/cmd/services/m3comparator/main/filterer.go
#	src/cmd/services/m3comparator/main/main.go
#	src/cmd/services/m3comparator/main/querier.go
#	src/cmd/services/m3comparator/main/querier_test.go
# Conflicts:
#	src/cmd/services/m3comparator/main/series_load_handler.go
#	src/cmd/services/m3comparator/main/series_load_handler_test.go
…mql-testdata

# Conflicts:
#	src/cmd/services/m3comparator/main/filterer.go
#	src/cmd/services/m3comparator/main/main.go
#	src/cmd/services/m3comparator/main/querier.go
#	src/cmd/services/m3comparator/main/querier_test.go
#	src/cmd/services/m3comparator/main/series_load_handler.go
#	src/cmd/services/m3comparator/main/series_load_handler_test.go
…-promql-testdata

# Conflicts:
#	src/cmd/services/m3comparator/main/querier.go
#	src/cmd/services/m3comparator/main/querier_test.go
…mql-testdata

# Conflicts:
#	src/cmd/services/m3comparator/main/querier.go
#	src/cmd/services/m3comparator/main/querier_test.go
…mql-testdata

# Conflicts:
#	src/cmd/services/m3comparator/main/querier_test.go
Co-authored-by: Linas Medžiūnas <linasm@users.noreply.github.com>
Co-authored-by: Linas Medžiūnas <linasm@users.noreply.github.com>
Co-authored-by: Linas Medžiūnas <linasm@users.noreply.github.com>
@gediminasgu gediminasgu marked this pull request as draft June 11, 2020 13:28
Gediminas added 2 commits June 11, 2020 17:49
# Conflicts:
#	src/query/api/v1/handler/prometheus/native/common.go
#	src/query/test/m3comparator_client.go
#	src/query/test/m3query_client.go
#	src/query/test/test.go
#	src/query/test/testdata/functions.test
#	src/query/test/testdata/histograms.test
#	src/query/test/testdata/legacy.test
Comment on lines +445 to +452

onlynans := true
for a := 0; a < length; a++ {
if !math.IsNaN(vals.ValueAt(a)) {
onlynans = false
break
}
}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Should this return the last non nan value, or explicitly 0 if there are trailing nans?

@codecov
Copy link
Copy Markdown

codecov Bot commented Sep 11, 2023

Codecov Report

❌ Patch coverage is 66.66667% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 71.6%. Comparing base (2c4f65c) to head (83a36f7).
⚠️ Report is 1420 commits behind head on master.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff            @@
##           master   #2402     +/-   ##
========================================
- Coverage    71.6%   71.6%   -0.1%     
========================================
  Files        1052    1052             
  Lines       93053   92934    -119     
========================================
- Hits        66702   66601    -101     
+ Misses      21891   21887      -4     
+ Partials     4460    4446     -14     
Flag Coverage Δ
aggregator 76.4% <ø> (+<0.1%) ⬆️
cluster 84.8% <ø> (ø)
collector 82.8% <ø> (ø)
dbnode 79.1% <ø> (+<0.1%) ⬆️
m3em 74.4% <ø> (ø)
m3ninx 73.1% <ø> (ø)
m3nsch 51.1% <ø> (ø)
metrics 17.3% <ø> (ø)
msg 75.2% <ø> (ø)
query 67.9% <66.6%> (-0.2%) ⬇️
x 81.8% <ø> (-0.1%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2c4f65c...83a36f7. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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.

4 participants