Added java.lang.Byte and java.lang.Short Valuefier support #9134
Conversation
|
Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually? |
|
Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually? |
|
We have a serious conflict of interest here. On the one hand we can't introduce valuefier convertors for every possible type that may enter Logstash from the various technologies used by plugins when they create a field/value combination. This implies that it is up to the plugin to do the normalisation, for example, from JDBC sql types. On the other hand, given that these are standard Java types, Byte and Short should probably be added. @original-brownbear WDYT? |
|
Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually? |
|
Jenkins please test this |
I agree, |
| @@ -112,6 +112,8 @@ private static Object fallbackConvert(final Object o, final Class<?> cls) { | |||
| converters.put(RubyBignum.class, IDENTITY); | |||
| converters.put(RubyBigDecimal.class, IDENTITY); | |||
| converters.put(String.class, input -> RubyUtil.RUBY.newString((String) input)); | |||
| converters.put(Byte.class, input -> RubyUtil.RUBY.newString(input.toString())); | |||
There was a problem hiding this comment.
@Twinkiee why don't we cast to int here and use LONG_CONVERTER as well? Seems more logical to me since a byte is a numeric type and not necessarily equivalent to a char in a String.
There was a problem hiding this comment.
I agree with @original-brownbear. At the local Event level a byte is a number.
However, I looked at the IBM MQ Java docs. I wanted to know if the byte is considered a ASCII character or a numeric value.
This header MQIIH can return a char for TranState.
I saw others that returned byte[].
Via the CodedCharSetId, messages can be in UTF8 or CCS-2 (and variants).
@Twinkiee
Given that, at the event level, we don't know what the byte represents, it seems like the jms plugin is a better place to code for this.
There was a problem hiding this comment.
@original-brownbear @guyboertje
It seems reasonable to me as well to convert the Byte as int as a general rule. That's how the java.lang.Byte#toString() threats the internal value anyway so there's really no point to behave differently.
I could/should've coded it in the first place, my bad.
On the other hand I'll try to investigate if it's actually possible for the IBM MQ to return a byte which is actually a char.
As far as I know, that's not the case. Sorry if it caused any confusion.
There was a problem hiding this comment.
Any progress on your investigation of IBM MQ char as byte investigation?
There was a problem hiding this comment.
@guyboertje
Hey, sorry for the late reply.
No, by my tests and investigations I couldn't find a byte converted into a char.
There was a problem hiding this comment.
@Twinkiee
Please make the update that @original-brownbear suggests then we can move this forward.
Faced this issue when dequeuing messages from an IBM MQ queue.
I added a couple more converters for java.lang.Byte and java.lang.Short which happened to be produced when the Logstash JMS plugin was deserializing the message headers.
Could not provide a meaningful (i.e. not trivial) unit test but I'd add one if asked/required.