Add numeric operators for filters.#1323
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1323 +/- ##
==========================================
+ Coverage 43.51% 43.58% +0.06%
==========================================
Files 305 307 +2
Lines 32864 32945 +81
==========================================
+ Hits 14301 14359 +58
- Misses 17643 17665 +22
- Partials 920 921 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| // NotMatch stores the glob that a given attribute MUST NOT match to let the record pass | ||
| NotMatch string `yaml:"not_match"` | ||
| // Numerical comparison for e.g. http.code or timings | ||
| GreaterThan float64 `yaml:"greater_than"` |
There was a problem hiding this comment.
What do you think about using *float64 instead of float64? In that case we can tell if the option is not set and allow us to filter on 0. We often have cases where maybe by error we cannot capture the http.code and we write 0, so I can see folks wanting to set a filter on that.
| m.Negate = true | ||
| default: | ||
| // use match-all as dummy for numeric-only comparisons | ||
| m.Glob = glob.MustCompile("*") |
There was a problem hiding this comment.
I think we need to add some statement that checks if none of these are set and report an error.
| // NotMatch stores the glob that a given attribute MUST NOT match to let the record pass | ||
| NotMatch string `yaml:"not_match"` | ||
| // Numerical comparison for e.g. http.code or timings | ||
| GreaterThan float64 `yaml:"greater_than"` |
There was a problem hiding this comment.
What do you think about making these *float64? Like this
GreaterThan *float64 `yaml:"greater_than,omitempty"`
That way we can test for the 0 port, 0 status code etc. Sometimes bugs in the eBPF code prevent us from finding the correct connection information so folks can filter on 0 server port etc.
There was a problem hiding this comment.
Actually I also wonder if we can make them ints? Technically, eBPF doesn't support floating point operations, it's not in the instruction set, so we can never get float value from there.
There was a problem hiding this comment.
I was originally thinking about floats for timings. On a closer look after your comment, I see that timings are either a float as string ("1.2ms") or int in µs, so we should be good here.
|
Btw: how are docs updated then? They live in https://github.com/open-telemetry/opentelemetry.io/tree/main/content/en/docs/zero-code/obi - right? How is that then synced with the OBI releases? |
980bc87 to
b671371
Compare
|
@pilhuhn I don't think there is yet any synchronization mechanism with the releases (we are still in a pre-1.0 status) You can just update there the documents |
grcevski
left a comment
There was a problem hiding this comment.
LGTM! This looks great now! I think the unit testing is sufficient, since we already have other test filters.
Addresses #1291
This introduces new numeric filter options
To be used e.g. like the following: