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

Add a way to test if a capture group has been defined in current scope #267

Open
finkr opened this issue Sep 4, 2019 · 6 comments
Open
Labels
enhancement This is considered a feature request, not currently guaranteed by the code or design today mtail-Language/VM Issues related to the mtail language, compiler, or VM

Comments

@finkr
Copy link

finkr commented Sep 4, 2019

Hi

When using a syntax like ((?P<response_size>\d+)|-) on a line containing a - instead of a digit, the variable response_time is undefined. When attempting to use that variable, mtail throws an error.

This error can be reproduced using apache_common.mtail and testdata/apache-common.log

...or this simplified test

Program

counter total

/^[a-z]+ ((?P<response_size>\d+)|-)$/ {

    $response_size > 0 {
            total = $response_size
    }
}

input log

test 99
test -

mtail output log

vm.go:92] test.mtail: Runtime error: strconv.ParseInt: parsing "": invalid syntax
vm.go:93] Error occurred at instruction 5 {s2i, <nil>}, originating in test.mtail at line 5
vm.go:95] Full input text from "log" was "test -"

I'am using mtail v3.0.0-rc33 (I tested rc16 and rc25 which have a similar behaviur)

@finkr
Copy link
Author

finkr commented Sep 4, 2019

I struggle to use that variable... all my attempts fails (I tried to use len(), or string()... but all fails.

As a workaround, I had to use an extra regex instead of checking the variable value

counter total

/^[a-z]+ ((?P<response_size>\d+)|-)$/ {

    # check if the current line contains a digit (not a dash)
    / \d+$/ {
            total = $response_size
    }
}

@jaqx0r
Copy link
Contributor

jaqx0r commented Sep 5, 2019 via email

@finkr
Copy link
Author

finkr commented Sep 5, 2019

In this particular case it would work... But there are some other case where it is not possible.

We need to be able to test a variable no matter if it match or not.

wmfgerrit pushed a commit to wikimedia/operations-puppet that referenced this issue May 27, 2020
mtail 3.0.0-rc35 does variable type detection during the program parsing phase.  in kind, $maxage
is being typed as an integer.

at runtime, encountering a log entry with no maxage match makes $maxage an empty string and
errors attempting to coerce it into an int (using strconv.ParseInt) and ultimately drops it.

this is a known bug: google/mtail#267

this cs mangles the match a bit so that mtail will not treat it like an integer, but a string

Bug: T251466
Change-Id: Ia764570b2d81c861e9fc7b5d52f56354006be6ec
@jaqx0r
Copy link
Contributor

jaqx0r commented Jan 13, 2021

The answer you came up with in #267 (comment) is the correct way to do this.

You've matched a field which is either a number or not a number. mtail can't parse the hyphen as a number, so it's up to you to tell it how to act. The inner condition will do the right thing at that point.

@jaqx0r jaqx0r changed the title Runtime error: strconv.ParseInt: parsing "": invalid syntax Can't parse a hyphen as a number Jan 13, 2021
@jaqx0r jaqx0r added the more-info-needed Waiting for an answer from the issue reporter label Jan 13, 2021
@jaqx0r
Copy link
Contributor

jaqx0r commented Jan 13, 2021

I can add this pattern to the documentation, possibly even make the error easier to understand; is there anything else you think mtail should do here?

@jaqx0r jaqx0r added enhancement This is considered a feature request, not currently guaranteed by the code or design today and removed more-info-needed Waiting for an answer from the issue reporter labels Jan 13, 2021
@jaqx0r
Copy link
Contributor

jaqx0r commented Jan 13, 2021

OK I think the feature to add is that we need a way to test if a capture group is defined.

The current programme that works is to test the surrounding capture group content, but I admit tis does not make the pgramme very readable.

@jaqx0r jaqx0r changed the title Can't parse a hyphen as a number Add a way to test if a capture group has been defined in current scope Jan 13, 2021
jaqx0r added a commit to jaqx0r/mtail that referenced this issue Jan 13, 2021
@jaqx0r jaqx0r added Language mtail-Language/VM Issues related to the mtail language, compiler, or VM and removed mtail-Language labels Mar 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement This is considered a feature request, not currently guaranteed by the code or design today mtail-Language/VM Issues related to the mtail language, compiler, or VM
Projects
None yet
Development

No branches or pull requests

2 participants