Issue with logstash 2.3 and coordinates returned by geoip filter #4961
Comments
This mean the value is out of bound with what ES want for a geocoords value, which is -90,90, and -180,180. So I think the geoip is returning a wrong value for one of the IP, but I would expect this IP to have the same behavior in LS 2.2.1 and LS 2.3. I have a few questions:
I think this is a rogue IP and the geoip return a bad result for it. |
|
I am not using the default database for geoip, I'm using the most recent one provided by maxmind. I want to remind you that this database works perfectly fine on 2.2.1. You are correct that the traces I provided show two different locations - that's the problem. The geoip filter is returning a valid latitude and longitude to logstash, and when logstash tries to send the location to elastic it has been changed to an invalid value. The IP is irrelevant, I promise. The same data works fine in 2.2.1. I'll spare you my entire config: 208.67.222.222 In this example, 208.68.222.222 is the source IP address. The geoip filter returned valid coordinates, and when logstash tried to send to elastic the coordinates seem to have been changed? The exact same data, maxmind database and logstash config works fine in 2.2.1 |
|
I did a bit more investigation with this config: https://gist.github.com/dbb64f216f010c5dbef09a1419b720dc The rubydebug output of the event is a bit different between the two version, one appear to be a string and the other a float. |
|
I had no problem serializing the data for with the provided IP, Can you try my configuration with your data? Any other place in your configuration that mutate the |
|
I'm not sure if it's the same problem but geoip filter Test Result |
|
@qk4l I'm sorry for your issues here, are you able to test this VS a fresh ES instance? my guess here would be a wrong mapping. As I understand from:
|
|
@arizonawayfarer I also did a few test with your issue and was unable to reproduce the error, my current idea is that there is something in your config that might have a bug and actually mutating the input values. In order to provide more clever solution, it would be super nice if you can share the full configuration of yours, like this we will be able to discard any other issues and narrow the problem down. Thanks a lot for your time and contribution. |
|
|
Here's the config for the logstash instance that collects the messages and puts them into MQ |
|
I guess I don't understand how it can be a configuration issue when this same config works perfect with logstash 2.2 |
|
I don't say is a config issue, but might be (I guess so) it might be a bug in a plugin used in your config. just thinking out loud as we had problems trying to reproduce 👍 |
|
thanks a lot for your patience and help on trying to narrow this down! |
|
A quick look at the configuration, I don't see anything suspect. Will try more testing with it ASAP. |
|
Here's a list of my installed plugins: |
|
Wanted to post an update, with 2.3.1 it looks like valid coordinates are being sent - however elastic is still returning a 400. I can't find the invalid coordinates this time in the message, but elastic is complaining about them in the src_ip_coords field. Come to think of it, I'm not sure I checked the elastic log last when 2.3 was failing, and I might need to punch myself. This may be an issue with elastic? Another theory I have is that the coordinates are being encapsulated in quotes, and that could be throwing elastic off? I'm running elasticsearch 2.3.1 Coordinates being sent to elastic: Stack trace from elastic: |
|
I think that the coordinates being encapsulated in quotes is it. In 2.2.1 the field values containing coordinates aren't being encapsulated in quotes. from 2.2.1: |
|
If I convert the offending field to a float using a mutate block, it goes into elastic just fine. I think this pretty much confirms the quoting issue? |
|
@arizonawayfarer sound like it, I dont think we have made significant change in the geoip filter between the two releases. This might be a side effect of the java event. Did you try to use 2.3.1 to see if you still have the problem? |
|
Yep, same thing happens in 2.3.1. The problem isn't in the geoip filter. I think that the problem is whatever is taking the values returned by geoip and converting them into strings when the new fields are added. It doesn't look like the geoip filter is actually adding the new fields - something else is doing that. And it's talking the floats returned by geoip and making them strings. I don't really know how this stuff works, so I'm just fumbling my way around now trying to understand the workflow. I'm currently looking at string_interpolation.rb - that appears to do conversions? |
|
Found the issue by doing a diff between the two case value
when nil
"%{#{@key}}"
when Array
value.join(",")
when Hash
LogStash::Json.dump(value)
else
# Make sure we dont work on the refence of the value
# The Java Event implementation was always returning a string.
"#{value}"
end
endWe coerce the data in a string. |
|
We need to add a special case when its a Float/Fixnum. |
|
@colinsurprenant I would like to have your input on this, We have made a change to the What should we do in that case, extend the switch case to return the Fixnum/float and everything else as string? We will need to implement the same logic in the java event. |
|
So if I recap
So it seems @ph's fix proposal would fix this and respect the previous behaviour. From a higher level perspective though, I find it confusing that something like |
|
I think that from a user perspective, it should be one way or the other. My two cents? I've always kind of thought that the quotes were superfluous when referencing variables in a config. Encapsulating a variable in quotes, or not doing it shouldn't infer the datatype of the variable. The data type of the variable should do that. I think that having the case statement is the right track, if it's a string return a quoted value. If not, don't. Don't let the config dictate that. |
|
@arizonawayfarer yeah but it becomes confusing is situation where you use interpolation within strings (which is very common) for examples: |
|
and I believe what I described in my previous comment is consistent with most/all string interpolation implementations in different programming languages. Again, for the sake of compatibility with prior behaviour it seems we should move forward with @ph's fix but my question here is more in terms of moving forward and more particularly in the context of an upcoming major version release where breaking compatibility is possible. |
|
@colinsurprenant thanks for jumping in, I also find it strange that string interpolation doesn't always return a string. For me, this syntax is similar to what I think for 2.3.2, we should apply the fix to make it backward compatible. But for a major release I am not sure we should support that. II will create the patch for fix the issue in 2.3.X and I will create an issue for |
|
note that currently the no-quote string interpolation syntax |
The change introduced with elastic#4592 to deal with reference had a side effect to break existing configuration when using fieldref accessing a Fixnum or a Float. The problem was instead of returning the original type , the value was converted to a string. This string could break some mappings that were expecting a float. Fixes elastic#4961
|
OK, I have created a patch which should be reviewed soon and created #5114 to see if we drop support for that in 5.0. |
|
@arizonawayfarer @colinsurprenant I found out this #5114 (comment) Its a regression yes but from 1.4.2 -> 1.5.X |
The change introduced with elastic#4592 to deal with reference had a side effect to break existing configuration when using fieldref accessing a Fixnum or a Float. The problem was instead of returning the original type , the value was converted to a string. This string could break some mappings that were expecting a float. Fixes elastic#4961
The change introduced with elastic#4592 to deal with reference had a side effect to break existing configuration when using fieldref accessing a Fixnum or a Float. The problem was instead of returning the original type , the value was converted to a string. This string could break some mappings that were expecting a float. Fixes elastic#4961
The change introduced with elastic#4592 to deal with reference had a side effect to break existing configuration when using fieldref accessing a Fixnum or a Float. The problem was instead of returning the original type , the value was converted to a string. This string could break some mappings that were expecting a float. Fixes elastic#4961
The change introduced with #4592 to deal with reference had a side effect to break existing configuration when using fieldref accessing a Fixnum or a Float. The problem was instead of returning the original type , the value was converted to a string. This string could break some mappings that were expecting a float. Fixes #4961 Fixes #5113
The change introduced with #4592 to deal with reference had a side effect to break existing configuration when using fieldref accessing a Fixnum or a Float. The problem was instead of returning the original type , the value was converted to a string. This string could break some mappings that were expecting a float. Fixes #4961 Fixes #5113
|
We will revert this changes for 2.3.2, so using |
I've run into an issue using the geoip filter in logstash 2.3 where the plugin is returning valid coordinates after looking up an IP address, but when logstash tries to send the data to elastic the coordinates have been changed to an invalid number. I added some additional logging to the geoip filter so I could see what was getting sent back to logstash.
When this happens, the invalid latitude that logstash tries to send to elastic is the same across all events until I restart logstash. After a restart the latitude changes to a new invalid value.
So this is getting returned to logstash from the geoip filter:
And then this is what happens when logstash tries to send to elastic:
I can't for the life of me figure out where the invalid latitude is coming from. I've removed actual IP addresses to protect the innocent, but the src_ip field contains a valid public IP address that is successfully filtered by geoip in logstash 2.2.1 :)
The same data and logstash configuration works fine in 2.2.1, this is something new that presented with 2.3
The text was updated successfully, but these errors were encountered: