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.

Sign up for GitHub

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

Jump to bottom

add absent function #172

Closed
wants to merge 5 commits into from
Closed

add absent function #172

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
6556125
add absent function
siddhikhapare Feb 9, 2023
bacf185
test file modified
siddhikhapare Feb 11, 2023
edaa37f
add support for functions
siddhikhapare Feb 11, 2023
3443fc8
files modified
siddhikhapare Feb 12, 2023
75adcb2
absent function added
siddhikhapare Feb 16, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
34 changes: 34 additions & 0 deletions engine/engine_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1500,6 +1500,24 @@ func TestQueriesAgainstOldEngine(t *testing.T) {
end: time.Unix(3000, 0),
step: 2 * time.Second,
},
{
name: "absent with present series",
load: `load 30s
absent{pod="nginx-1", series="1"} 0
absent{pod="nginx-2", series="1"} 0`,
query: "absent(metric)",
Copy link
Contributor

Choose a reason for hiding this comment

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

This test case seems wrong itself. Your metric name is absent but you are calling absent for metric. metric is not present in your test data.

start: time.Unix(0, 0),
end: time.Unix(3000, 0),
step: 10 * time.Second,
},
{
name: "absent with non-present series",
load: "",
query: "absent(non_existent_metric)",
start: time.Unix(0, 0),
end: time.Unix(3000, 0),
step: 10 * time.Second,
},
}

disableOptimizerOpts := []bool{true, false}
Expand Down Expand Up @@ -2431,6 +2449,22 @@ func TestInstantQuery(t *testing.T) {
http_requests_total{pod="nginx-2", series="1"} -10+1x50`,
query: "sgn(http_requests_total)",
},
{
name: "absent with present series",
load: `load 30s
absent{pod="nginx-1", series="1"} 0
absent{pod="nginx-2", series="1"} 0`,
query: "absent(metric)",
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

queryTime: time.Now(),
sortByLabels: true,
},
{
name: "absent with non-present series",
load: "",
query: "absent(non_existent_metric)",
queryTime: time.Now(),
sortByLabels: false,
},
}

disableOptimizerOpts := []bool{true, false}
Expand Down
20 changes: 18 additions & 2 deletions execution/function/functions.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ import (
"fmt"
"math"
"time"

"github.com/efficientgo/core/errors"
"github.com/efficientgo/core/errors"
"github.com/prometheus/prometheus/model/histogram"
"github.com/prometheus/prometheus/model/labels"
"github.com/prometheus/prometheus/promql"
Expand Down Expand Up @@ -434,6 +434,22 @@ var Funcs = map[string]FunctionCall{
return float64(t.Year())
})
},
"absent": func(f FunctionArgs) promql.Sample {
if len(f.Points) == 0 {
return promql.Sample{
Metric: f.Labels,
Point: promql.Point{
T: f.StepTime,
V: 1,
},
}
}

return promql.Sample{
Metric: f.Labels,
Point: promql.Point{},
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this is the correct. absent checks whether the series exists ot not, not samples. When the series exists, I think what you want is to drop the whole series, not returning an empty point. Implementing a function like this couldn't achieve the goal of dropping the series right now, you have to refactor the framework to do that.

Probably a dedicated absent operator is easier.

Copy link
Author

Choose a reason for hiding this comment

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

@yeya24 Thank you for the guidance. I will go through @MichaHoffmann implementation to understand it.

},
}

func NewFunctionCall(f *parser.Function) (FunctionCall, error) {
Expand Down