-
Notifications
You must be signed in to change notification settings - Fork 18
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
Implement filtering mechanism for cmetrics context #193
Conversation
Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>
Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>
Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>
Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>
fa21f6e
to
a140738
Compare
Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>
Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>
Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>
3071c75
to
05baacd
Compare
Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>
Because (con)cat(enate) implies copying the pointer/context. Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>
Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>
I've done for the suggested changes. |
src/cmt_filter.c
Outdated
struct cmt_map_label *label; | ||
size_t label_key_size = 0; | ||
|
||
if (label_key != NULL) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if there is a chance this can be NULL the code below will not work. Either the check is not necessary or we just return with -1 if is NULL
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a chance to become NULL. Because we are able to specify an external function only here.
And this should be injected via compare_ctx and callback function.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
label_key is only needed for prefix matching case. I added a clause which checks for CMT_FILTER_PREFIX flag.
src/cmt_filter.c
Outdated
* The length of string is changed by the callback and not determined by fqname. | ||
*/ | ||
if (compare_ctx != NULL && compare != NULL) { | ||
return compare(compare_ctx, src->fqname, strlen(src->fqname)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is fqname an sds ?
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Once I rethought this, I realized this could be calculated before it's needed. So, prefix matching case and using an external function case should be able to calculate size of fqname. So, the previous my comment is wrong. Thanks for your suggestion.
Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>
Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>
If label_key is NULL and CMT_FILTER_PREFIX specified case, SEGV will happen. So, we need to return imetteately for this case. Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>
3a3dcc6
to
58f8951
Compare
Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>
Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>
I implemented filtering mechanism which is mainly targeted for full qualified name of metrics (fqname).
Maybe, we need to implement filtering mechanism for label values but traversing values of labels are more complex rather than just searching with keys of labels.
Also, cmetrics should provide a capability to inject external callback function which uses Onigmo contexts that are mainly used in filter_grep or perhaps processor_grep?