You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@druid.apache.org by noam shaish <no...@gmail.com> on 2021/04/16 22:28:43 UTC

Adding support to Kafka events keys

Hi,
I would like to try and add a InputFormat for Kafka to support also fields coming from the event key.
In my scenario there are to options:
1. both key and value are json
2. key is delimited string and the value is json.

Would such a feature will be welcome for contribution? or should I keep on my own fork?

Thanks,
Noam 

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@druid.apache.org
For additional commands, e-mail: dev-help@druid.apache.org


Re: Adding support to Kafka events keys

Posted by noam shaish <no...@gmail.com>.
Hi Gian,

I have seen the mention code but I have several points I am currently struggling with:
1. Currently InputFormats are not very composable so I cannot reuse functionality from the existing InputFormats, so I have to replicate the functionality in my implementation (I really dont like to do that).
I could extract the common functionality to some common utilities but that will yield a big refactoring.
2.  Since I receive the records as "InputEntity" which is generic I am using "IntermediateRowParsingReader" to cast the records to KeyValueEntity, but what should be the behaviour for records which are not key value? 
I could Just return an "empty" key that will provide no additional fields but thats doesnt feel right. a better approach was to limit this combination from existing, I guess this can be achieved in the UI part but thats out of my skillset (backend developer.....)

Regarding your comments:
1. Thanks for the pointer, I thought by simply defining dedicated Input formats for the 2 use cases I require and generalize later. first get it working :)
2 + 3.  I could solve this by having a structured spec that will have "sub specs" for the key and value.
But as I mentioned before InputFormats are not composable. Meaning I need to replicate all specs if I wont make a huge refactor. Therefor I thought to start with a dedicated formats:
JsonJson, DelimitedJson at start to cover the use cases I wish to cover for now.
Than I guess some pattern will reveal itself for any addition :)

On 2021/04/21 15:40:28, Gian Merlino <gi...@apache.org> wrote: 
> Hey Noam,
> 
> I think this would certainly be useful, and thank you for your interest in
> contributing!
> 
> I think the toughest part will be designing a good API (meaning: what would
> users specify in the kafka supervisor json spec in order to activate and
> configure this feature?). So a good way to proceed would be to propose some
> API, gather some community feedback on the design of the API, and then
> start working on a patch.
> 
> Some thoughts on API design:
> 
> 1) https://github.com/apache/druid/pull/10730 adds some related
> functionality that you would want to hook into. This patch added Java APIs
> that can be used in extensions, but didn't add any JSON APIs that can be
> used by regular users. But you could build some JSON APIs on top of this.
> 
> 2) Some keys are "formatted" (like the examples you gave: json and
> delimited). Formatted keys should be parsed and fields extracted from them
> somehow, using their own InputFormat. Maybe we should call it the
> "keyInputFormat". We need to figure out what semantics make the most sense
> for presenting the parsed key to later stages of the system (which expect a
> single namespace). Merging the parsed key map with the parsed value map
> seems like a bad idea, since there might be field name collisions. So maybe
> we should prefix them with some string like "__key.". There could still be
> collisions, but they'd be less likely if we choose an uncommon prefix. At
> some point, we may also need to let users specify their own prefix, or even
> something fancier like an explicit mapping. But I think we won't need that
> feature on day 1.
> 
> 3) There are also unformatted keys that might be simple strings or byte
> arrays. These unformatted keys should become a single field. I’m not sure
> which is more prevalent, or which one we should build first, but I think
> ultimately we’ll want to support both styles.
> 
> On Fri, Apr 16, 2021 at 3:36 PM noam shaish <no...@gmail.com> wrote:
> 
> > Hi,
> > I would like to try and add a InputFormat for Kafka to support also fields
> > coming from the event key.
> > In my scenario there are to options:
> > 1. both key and value are json
> > 2. key is delimited string and the value is json.
> >
> > Would such a feature will be welcome for contribution? or should I keep on
> > my own fork?
> >
> > Thanks,
> > Noam
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: dev-unsubscribe@druid.apache.org
> > For additional commands, e-mail: dev-help@druid.apache.org
> >
> >
> 

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@druid.apache.org
For additional commands, e-mail: dev-help@druid.apache.org


Re: Adding support to Kafka events keys

Posted by Xavier Léauté <xa...@confluent.io.INVALID>.
Hi Noam, see my answers in-line below.

Would an option in the spec "includeHeaders" make sense?
> Then user could indicate he wish to include headers as fields.
>

I don't think that would be enough. The header name is a string, but the
header value is a byte array.
So we would still need to be able to define a spec for each individual
header.
In that sense a header is no different than the key, except there are
multiple, and each has a name.

Now, in many cases there will be a whole range of headers that all use the
same spec, or will be simple utf-8 encoded string.
In this case we might want to have an easy way to apply a spec to multiple
headers, or pre-defined specs for common encodings.
There are standards out there such as CloudEvents, which leverage Kafka
headers already, so it might be useful to think how we would
want to express those in Druid (see
https://github.com/cloudevents/spec/blob/v1.0.1/kafka-protocol-binding.md)

The part that might be confusing in this setup that the input specs
> existing today have repetition in them like timestampSpec which make sense
> only in the root Spec, but that could be simply ignored if provided (Not
> very friendly but again overcoming it will require a bigger refactoring)
>

Agree, but it might be worth digging a little deeper here. Keep in mind
that Kafka records define their own timestamp, so it might be useful to
allow users to specify to use the record timestamp when needed.
This is especially useful when consuming the output of Kafka-Streams
applications, which by default only output the Kafka timestamp, and don't
include it in the record value or key.
Ideally, we wouldn't want to have to make drastic API changes to support
that in the future.

Re: Adding support to Kafka events keys

Posted by noam shaish <no...@gmail.com>.
Hi Xavier,
Thanks for the pointer,
Would an option in the spec "includeHeaders" make sense?
Then user could indicate he wish to include headers as fields.

My idea for API for the spec was:
{
 "timestampSpec": ...,
 "dimensionsSpec": ...,
 ...
  "includeHeaders": true,
  "headersPrefix": "hd_",
  "keyPrefix": "k_",
  "keySpec": { 
    ... an existing spec for key input format, for example JSONParseSpec (or a sub set of the existing one)
  },
  "valueSpec": {
   ... an existing spec for key input format, for example JSONParseSpec (or a sub set of the existing one)
  }
  "keepNullColumns": false,
  ...
}

The part that might be confusing in this setup that the input specs existing today have repetition in them like timestampSpec which make sense only in the root Spec, but that could be simply ignored if provided (Not very friendly but again overcoming it will require a bigger refactoring)

On 2021/04/21 17:13:45, Xavier Léauté <xa...@confluent.io.INVALID> wrote: 
> Thanks Noam,
> 
> I think this would be a great addition. We should not limit this to keys
> though, and include Kafka record headers as well, since the semantics are
> similar.
> Header names are already well-defined strings, so an API for keys would
> likely apply easily to headers as well.
> 
> As Gian mentioned, the support to access those Kafka record fields from
> InputFormat is already there, but we lack a good user-facing API,
> so if you would like to help figure that out that'd be awesome.
> 
> - Xavier
> 
> On Wed, Apr 21, 2021 at 8:40 AM Gian Merlino <gi...@apache.org> wrote:
> 
> > Hey Noam,
> >
> > I think this would certainly be useful, and thank you for your interest in
> > contributing!
> >
> > I think the toughest part will be designing a good API (meaning: what would
> > users specify in the kafka supervisor json spec in order to activate and
> > configure this feature?). So a good way to proceed would be to propose some
> > API, gather some community feedback on the design of the API, and then
> > start working on a patch.
> >
> > Some thoughts on API design:
> >
> > 1) https://github.com/apache/druid/pull/10730 adds some related
> > functionality that you would want to hook into. This patch added Java APIs
> > that can be used in extensions, but didn't add any JSON APIs that can be
> > used by regular users. But you could build some JSON APIs on top of this.
> >
> > 2) Some keys are "formatted" (like the examples you gave: json and
> > delimited). Formatted keys should be parsed and fields extracted from them
> > somehow, using their own InputFormat. Maybe we should call it the
> > "keyInputFormat". We need to figure out what semantics make the most sense
> > for presenting the parsed key to later stages of the system (which expect a
> > single namespace). Merging the parsed key map with the parsed value map
> > seems like a bad idea, since there might be field name collisions. So maybe
> > we should prefix them with some string like "__key.". There could still be
> > collisions, but they'd be less likely if we choose an uncommon prefix. At
> > some point, we may also need to let users specify their own prefix, or even
> > something fancier like an explicit mapping. But I think we won't need that
> > feature on day 1.
> >
> > 3) There are also unformatted keys that might be simple strings or byte
> > arrays. These unformatted keys should become a single field. I’m not sure
> > which is more prevalent, or which one we should build first, but I think
> > ultimately we’ll want to support both styles.
> >
> > On Fri, Apr 16, 2021 at 3:36 PM noam shaish <no...@gmail.com> wrote:
> >
> > > Hi,
> > > I would like to try and add a InputFormat for Kafka to support also
> > fields
> > > coming from the event key.
> > > In my scenario there are to options:
> > > 1. both key and value are json
> > > 2. key is delimited string and the value is json.
> > >
> > > Would such a feature will be welcome for contribution? or should I keep
> > on
> > > my own fork?
> > >
> > > Thanks,
> > > Noam
> > >
> > > ---------------------------------------------------------------------
> > > To unsubscribe, e-mail: dev-unsubscribe@druid.apache.org
> > > For additional commands, e-mail: dev-help@druid.apache.org
> > >
> > >
> >
> 

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@druid.apache.org
For additional commands, e-mail: dev-help@druid.apache.org


Re: Adding support to Kafka events keys

Posted by Xavier Léauté <xa...@confluent.io.INVALID>.
Thanks Noam,

I think this would be a great addition. We should not limit this to keys
though, and include Kafka record headers as well, since the semantics are
similar.
Header names are already well-defined strings, so an API for keys would
likely apply easily to headers as well.

As Gian mentioned, the support to access those Kafka record fields from
InputFormat is already there, but we lack a good user-facing API,
so if you would like to help figure that out that'd be awesome.

- Xavier

On Wed, Apr 21, 2021 at 8:40 AM Gian Merlino <gi...@apache.org> wrote:

> Hey Noam,
>
> I think this would certainly be useful, and thank you for your interest in
> contributing!
>
> I think the toughest part will be designing a good API (meaning: what would
> users specify in the kafka supervisor json spec in order to activate and
> configure this feature?). So a good way to proceed would be to propose some
> API, gather some community feedback on the design of the API, and then
> start working on a patch.
>
> Some thoughts on API design:
>
> 1) https://github.com/apache/druid/pull/10730 adds some related
> functionality that you would want to hook into. This patch added Java APIs
> that can be used in extensions, but didn't add any JSON APIs that can be
> used by regular users. But you could build some JSON APIs on top of this.
>
> 2) Some keys are "formatted" (like the examples you gave: json and
> delimited). Formatted keys should be parsed and fields extracted from them
> somehow, using their own InputFormat. Maybe we should call it the
> "keyInputFormat". We need to figure out what semantics make the most sense
> for presenting the parsed key to later stages of the system (which expect a
> single namespace). Merging the parsed key map with the parsed value map
> seems like a bad idea, since there might be field name collisions. So maybe
> we should prefix them with some string like "__key.". There could still be
> collisions, but they'd be less likely if we choose an uncommon prefix. At
> some point, we may also need to let users specify their own prefix, or even
> something fancier like an explicit mapping. But I think we won't need that
> feature on day 1.
>
> 3) There are also unformatted keys that might be simple strings or byte
> arrays. These unformatted keys should become a single field. I’m not sure
> which is more prevalent, or which one we should build first, but I think
> ultimately we’ll want to support both styles.
>
> On Fri, Apr 16, 2021 at 3:36 PM noam shaish <no...@gmail.com> wrote:
>
> > Hi,
> > I would like to try and add a InputFormat for Kafka to support also
> fields
> > coming from the event key.
> > In my scenario there are to options:
> > 1. both key and value are json
> > 2. key is delimited string and the value is json.
> >
> > Would such a feature will be welcome for contribution? or should I keep
> on
> > my own fork?
> >
> > Thanks,
> > Noam
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: dev-unsubscribe@druid.apache.org
> > For additional commands, e-mail: dev-help@druid.apache.org
> >
> >
>

Re: Adding support to Kafka events keys

Posted by Gian Merlino <gi...@apache.org>.
Hey Noam,

I think this would certainly be useful, and thank you for your interest in
contributing!

I think the toughest part will be designing a good API (meaning: what would
users specify in the kafka supervisor json spec in order to activate and
configure this feature?). So a good way to proceed would be to propose some
API, gather some community feedback on the design of the API, and then
start working on a patch.

Some thoughts on API design:

1) https://github.com/apache/druid/pull/10730 adds some related
functionality that you would want to hook into. This patch added Java APIs
that can be used in extensions, but didn't add any JSON APIs that can be
used by regular users. But you could build some JSON APIs on top of this.

2) Some keys are "formatted" (like the examples you gave: json and
delimited). Formatted keys should be parsed and fields extracted from them
somehow, using their own InputFormat. Maybe we should call it the
"keyInputFormat". We need to figure out what semantics make the most sense
for presenting the parsed key to later stages of the system (which expect a
single namespace). Merging the parsed key map with the parsed value map
seems like a bad idea, since there might be field name collisions. So maybe
we should prefix them with some string like "__key.". There could still be
collisions, but they'd be less likely if we choose an uncommon prefix. At
some point, we may also need to let users specify their own prefix, or even
something fancier like an explicit mapping. But I think we won't need that
feature on day 1.

3) There are also unformatted keys that might be simple strings or byte
arrays. These unformatted keys should become a single field. I’m not sure
which is more prevalent, or which one we should build first, but I think
ultimately we’ll want to support both styles.

On Fri, Apr 16, 2021 at 3:36 PM noam shaish <no...@gmail.com> wrote:

> Hi,
> I would like to try and add a InputFormat for Kafka to support also fields
> coming from the event key.
> In my scenario there are to options:
> 1. both key and value are json
> 2. key is delimited string and the value is json.
>
> Would such a feature will be welcome for contribution? or should I keep on
> my own fork?
>
> Thanks,
> Noam
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@druid.apache.org
> For additional commands, e-mail: dev-help@druid.apache.org
>
>