You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@flume.apache.org by Tim Bacon <ti...@gmail.com> on 2013/04/29 10:43:02 UTC
Review Request: FLUME-2015 ElasticSearchSink: need access to
IndexRequestBuilder instance during flume event processing
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10835/
-----------------------------------------------------------
Review request for Flume.
Description
-------
This change adds an ElasticSearchIndexRequestBuilderFactory interface to allow users of an ElasticSearchSink to have greater control over the actual indexing of events that arrive at the sink. It is intended to meet the needs of:
- my own non-Kibana/non-logging-event use case
- the specific "allow for indexing an id-field" needs of FLUME-1972
- the (UTC) determination of the index to write to per FLUME-1782
This patch is backwards-compatible and imposes no changes on existing users of the sink.
This addresses bug FLUME-2015.
https://issues.apache.org/jira/browse/FLUME-2015
Diffs
-----
flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/AbstractElasticSearchIndexRequestBuilderFactory.java PRE-CREATION
flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchEventSerializer.java dc6a093
flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchIndexRequestBuilderFactory.java PRE-CREATION
flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchSink.java 1b3db14
flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/EventSerializerIndexRequestBuilderFactory.java PRE-CREATION
flume-ng-sinks/flume-ng-elasticsearch-sink/src/test/java/org/apache/flume/sink/elasticsearch/AbstractElasticSearchSinkTest.java 2edacdc
flume-ng-sinks/flume-ng-elasticsearch-sink/src/test/java/org/apache/flume/sink/elasticsearch/TestElasticSearchIndexRequestBuilderFactory.java PRE-CREATION
flume-ng-sinks/flume-ng-elasticsearch-sink/src/test/java/org/apache/flume/sink/elasticsearch/TestElasticSearchSink.java 94b95b1
Diff: https://reviews.apache.org/r/10835/diff/
Testing
-------
Unit tests added and run.
Patch applied and run successfully in my own flume test environment.
Thanks,
Tim Bacon
Re: Review Request: FLUME-2015 ElasticSearchSink: need access to
IndexRequestBuilder instance during flume event processing
Posted by Mike Percy <mp...@apache.org>.
> On May 8, 2013, 8:05 p.m., Mike Percy wrote:
> > flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchEventSerializer.java, line 47
> > <https://reviews.apache.org/r/10835/diff/1/?file=285702#file285702line47>
> >
> > This is an API-breaking change for existing plugins.
> >
> > Instead of this, a new interface should be created if this change needs to be made, and reflection (instanceof) should be used at the configuration layer to decide which serializer interface was implemented so that a separate codepath can be used.
instanceof, or isAssignableFrom, etc
- Mike
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10835/#review20339
-----------------------------------------------------------
On May 3, 2013, 12:24 a.m., Tim Bacon wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10835/
> -----------------------------------------------------------
>
> (Updated May 3, 2013, 12:24 a.m.)
>
>
> Review request for Flume and Israel Ekpo.
>
>
> Description
> -------
>
> This change adds an ElasticSearchIndexRequestBuilderFactory interface to allow users of an ElasticSearchSink to have greater control over the actual indexing of events that arrive at the sink. It is intended to meet the needs of:
> - my own non-Kibana/non-logging-event use case
> - the specific "allow for indexing an id-field" needs of FLUME-1972
> - the (UTC) determination of the index to write to per FLUME-1782
>
> This patch is backwards-compatible and imposes no changes on existing users of the sink.
>
>
> This addresses bug FLUME-2015.
> https://issues.apache.org/jira/browse/FLUME-2015
>
>
> Diffs
> -----
>
> flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/AbstractElasticSearchIndexRequestBuilderFactory.java PRE-CREATION
> flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchEventSerializer.java dc6a093
> flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchIndexRequestBuilderFactory.java PRE-CREATION
> flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchSink.java 1b3db14
> flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/EventSerializerIndexRequestBuilderFactory.java PRE-CREATION
> flume-ng-sinks/flume-ng-elasticsearch-sink/src/test/java/org/apache/flume/sink/elasticsearch/AbstractElasticSearchSinkTest.java 2edacdc
> flume-ng-sinks/flume-ng-elasticsearch-sink/src/test/java/org/apache/flume/sink/elasticsearch/TestElasticSearchIndexRequestBuilderFactory.java PRE-CREATION
> flume-ng-sinks/flume-ng-elasticsearch-sink/src/test/java/org/apache/flume/sink/elasticsearch/TestElasticSearchSink.java 94b95b1
>
> Diff: https://reviews.apache.org/r/10835/diff/
>
>
> Testing
> -------
>
> Unit tests added and run.
> Patch applied and run successfully in my own flume test environment.
>
>
> Thanks,
>
> Tim Bacon
>
>
Re: Review Request: FLUME-2015 ElasticSearchSink: need access to
IndexRequestBuilder instance during flume event processing
Posted by Tim Bacon <ti...@gmail.com>.
> On May 8, 2013, 8:05 p.m., Mike Percy wrote:
> > flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchEventSerializer.java, line 47
> > <https://reviews.apache.org/r/10835/diff/1/?file=285702#file285702line47>
> >
> > This is an API-breaking change for existing plugins.
> >
> > Instead of this, a new interface should be created if this change needs to be made, and reflection (instanceof) should be used at the configuration layer to decide which serializer interface was implemented so that a separate codepath can be used.
>
> Mike Percy wrote:
> instanceof, or isAssignableFrom, etc
This is not an API-breaking change for existing plugins: XContentBuilder (current return type) is a concrete class that implements the BytesStream interface (proposed / replacement return type).
- Tim
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10835/#review20339
-----------------------------------------------------------
On May 3, 2013, 12:24 a.m., Tim Bacon wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10835/
> -----------------------------------------------------------
>
> (Updated May 3, 2013, 12:24 a.m.)
>
>
> Review request for Flume and Israel Ekpo.
>
>
> Description
> -------
>
> This change adds an ElasticSearchIndexRequestBuilderFactory interface to allow users of an ElasticSearchSink to have greater control over the actual indexing of events that arrive at the sink. It is intended to meet the needs of:
> - my own non-Kibana/non-logging-event use case
> - the specific "allow for indexing an id-field" needs of FLUME-1972
> - the (UTC) determination of the index to write to per FLUME-1782
>
> This patch is backwards-compatible and imposes no changes on existing users of the sink.
>
>
> This addresses bug FLUME-2015.
> https://issues.apache.org/jira/browse/FLUME-2015
>
>
> Diffs
> -----
>
> flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/AbstractElasticSearchIndexRequestBuilderFactory.java PRE-CREATION
> flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchEventSerializer.java dc6a093
> flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchIndexRequestBuilderFactory.java PRE-CREATION
> flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchSink.java 1b3db14
> flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/EventSerializerIndexRequestBuilderFactory.java PRE-CREATION
> flume-ng-sinks/flume-ng-elasticsearch-sink/src/test/java/org/apache/flume/sink/elasticsearch/AbstractElasticSearchSinkTest.java 2edacdc
> flume-ng-sinks/flume-ng-elasticsearch-sink/src/test/java/org/apache/flume/sink/elasticsearch/TestElasticSearchIndexRequestBuilderFactory.java PRE-CREATION
> flume-ng-sinks/flume-ng-elasticsearch-sink/src/test/java/org/apache/flume/sink/elasticsearch/TestElasticSearchSink.java 94b95b1
>
> Diff: https://reviews.apache.org/r/10835/diff/
>
>
> Testing
> -------
>
> Unit tests added and run.
> Patch applied and run successfully in my own flume test environment.
>
>
> Thanks,
>
> Tim Bacon
>
>
Re: Review Request: FLUME-2015 ElasticSearchSink: need access to
IndexRequestBuilder instance during flume event processing
Posted by Mike Percy <mp...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10835/#review20339
-----------------------------------------------------------
Hi Tim, thanks for the patch!
If Edward is ok with these changes then I'm +1 on them, except for the API breakage below. We need to maintain backwards compatibility with the existing API implementation.
flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchEventSerializer.java
<https://reviews.apache.org/r/10835/#comment41900>
This is an API-breaking change for existing plugins.
Instead of this, a new interface should be created if this change needs to be made, and reflection (instanceof) should be used at the configuration layer to decide which serializer interface was implemented so that a separate codepath can be used.
- Mike Percy
On May 3, 2013, 12:24 a.m., Tim Bacon wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10835/
> -----------------------------------------------------------
>
> (Updated May 3, 2013, 12:24 a.m.)
>
>
> Review request for Flume and Israel Ekpo.
>
>
> Description
> -------
>
> This change adds an ElasticSearchIndexRequestBuilderFactory interface to allow users of an ElasticSearchSink to have greater control over the actual indexing of events that arrive at the sink. It is intended to meet the needs of:
> - my own non-Kibana/non-logging-event use case
> - the specific "allow for indexing an id-field" needs of FLUME-1972
> - the (UTC) determination of the index to write to per FLUME-1782
>
> This patch is backwards-compatible and imposes no changes on existing users of the sink.
>
>
> This addresses bug FLUME-2015.
> https://issues.apache.org/jira/browse/FLUME-2015
>
>
> Diffs
> -----
>
> flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/AbstractElasticSearchIndexRequestBuilderFactory.java PRE-CREATION
> flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchEventSerializer.java dc6a093
> flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchIndexRequestBuilderFactory.java PRE-CREATION
> flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchSink.java 1b3db14
> flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/EventSerializerIndexRequestBuilderFactory.java PRE-CREATION
> flume-ng-sinks/flume-ng-elasticsearch-sink/src/test/java/org/apache/flume/sink/elasticsearch/AbstractElasticSearchSinkTest.java 2edacdc
> flume-ng-sinks/flume-ng-elasticsearch-sink/src/test/java/org/apache/flume/sink/elasticsearch/TestElasticSearchIndexRequestBuilderFactory.java PRE-CREATION
> flume-ng-sinks/flume-ng-elasticsearch-sink/src/test/java/org/apache/flume/sink/elasticsearch/TestElasticSearchSink.java 94b95b1
>
> Diff: https://reviews.apache.org/r/10835/diff/
>
>
> Testing
> -------
>
> Unit tests added and run.
> Patch applied and run successfully in my own flume test environment.
>
>
> Thanks,
>
> Tim Bacon
>
>
Re: Review Request: FLUME-2015 ElasticSearchSink: need access to
IndexRequestBuilder instance during flume event processing
Posted by Mike Percy <mp...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10835/#review20428
-----------------------------------------------------------
Ship it!
+1 looks good!
I did not test this myself (although the unit tests pass) but I know you and Edward did so that's good enough for me. :)
- Mike Percy
On May 10, 2013, 10:08 a.m., Tim Bacon wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10835/
> -----------------------------------------------------------
>
> (Updated May 10, 2013, 10:08 a.m.)
>
>
> Review request for Flume and Israel Ekpo.
>
>
> Description
> -------
>
> This change adds an ElasticSearchIndexRequestBuilderFactory interface to allow users of an ElasticSearchSink to have greater control over the actual indexing of events that arrive at the sink. It is intended to meet the needs of:
> - my own non-Kibana/non-logging-event use case
> - the specific "allow for indexing an id-field" needs of FLUME-1972
> - the (UTC) determination of the index to write to per FLUME-1782
>
> This patch is backwards-compatible and imposes no changes on existing users of the sink.
>
>
> This addresses bug FLUME-2015.
> https://issues.apache.org/jira/browse/FLUME-2015
>
>
> Diffs
> -----
>
> flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/AbstractElasticSearchIndexRequestBuilderFactory.java PRE-CREATION
> flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchEventSerializer.java dc6a093
> flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchIndexRequestBuilderFactory.java PRE-CREATION
> flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchSink.java 1b3db14
> flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/EventSerializerIndexRequestBuilderFactory.java PRE-CREATION
> flume-ng-sinks/flume-ng-elasticsearch-sink/src/test/java/org/apache/flume/sink/elasticsearch/AbstractElasticSearchSinkTest.java 2edacdc
> flume-ng-sinks/flume-ng-elasticsearch-sink/src/test/java/org/apache/flume/sink/elasticsearch/TestElasticSearchIndexRequestBuilderFactory.java PRE-CREATION
> flume-ng-sinks/flume-ng-elasticsearch-sink/src/test/java/org/apache/flume/sink/elasticsearch/TestElasticSearchSink.java 94b95b1
>
> Diff: https://reviews.apache.org/r/10835/diff/
>
>
> Testing
> -------
>
> Unit tests added and run.
> Patch applied and run successfully in my own flume test environment.
>
>
> Thanks,
>
> Tim Bacon
>
>
Re: Review Request: FLUME-2015 ElasticSearchSink: need access to
IndexRequestBuilder instance during flume event processing
Posted by Tim Bacon <ti...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10835/
-----------------------------------------------------------
(Updated May 10, 2013, 10:08 a.m.)
Review request for Flume and Israel Ekpo.
Changes
-------
Changes from prior diff: ElasticSearchIndexRequestBuilderFactory interface should extend Configurable / ConfigurableComponent
Description
-------
This change adds an ElasticSearchIndexRequestBuilderFactory interface to allow users of an ElasticSearchSink to have greater control over the actual indexing of events that arrive at the sink. It is intended to meet the needs of:
- my own non-Kibana/non-logging-event use case
- the specific "allow for indexing an id-field" needs of FLUME-1972
- the (UTC) determination of the index to write to per FLUME-1782
This patch is backwards-compatible and imposes no changes on existing users of the sink.
This addresses bug FLUME-2015.
https://issues.apache.org/jira/browse/FLUME-2015
Diffs (updated)
-----
flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/AbstractElasticSearchIndexRequestBuilderFactory.java PRE-CREATION
flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchEventSerializer.java dc6a093
flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchIndexRequestBuilderFactory.java PRE-CREATION
flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchSink.java 1b3db14
flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/EventSerializerIndexRequestBuilderFactory.java PRE-CREATION
flume-ng-sinks/flume-ng-elasticsearch-sink/src/test/java/org/apache/flume/sink/elasticsearch/AbstractElasticSearchSinkTest.java 2edacdc
flume-ng-sinks/flume-ng-elasticsearch-sink/src/test/java/org/apache/flume/sink/elasticsearch/TestElasticSearchIndexRequestBuilderFactory.java PRE-CREATION
flume-ng-sinks/flume-ng-elasticsearch-sink/src/test/java/org/apache/flume/sink/elasticsearch/TestElasticSearchSink.java 94b95b1
Diff: https://reviews.apache.org/r/10835/diff/
Testing
-------
Unit tests added and run.
Patch applied and run successfully in my own flume test environment.
Thanks,
Tim Bacon
Re: Review Request: FLUME-2015 ElasticSearchSink: need access to
IndexRequestBuilder instance during flume event processing
Posted by Tim Bacon <ti...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10835/
-----------------------------------------------------------
(Updated May 3, 2013, 12:24 a.m.)
Review request for Flume and Israel Ekpo.
Changes
-------
Adding iekpo to the list of reviewers based on his review of FLUME-1782
Description
-------
This change adds an ElasticSearchIndexRequestBuilderFactory interface to allow users of an ElasticSearchSink to have greater control over the actual indexing of events that arrive at the sink. It is intended to meet the needs of:
- my own non-Kibana/non-logging-event use case
- the specific "allow for indexing an id-field" needs of FLUME-1972
- the (UTC) determination of the index to write to per FLUME-1782
This patch is backwards-compatible and imposes no changes on existing users of the sink.
This addresses bug FLUME-2015.
https://issues.apache.org/jira/browse/FLUME-2015
Diffs
-----
flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/AbstractElasticSearchIndexRequestBuilderFactory.java PRE-CREATION
flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchEventSerializer.java dc6a093
flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchIndexRequestBuilderFactory.java PRE-CREATION
flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchSink.java 1b3db14
flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/EventSerializerIndexRequestBuilderFactory.java PRE-CREATION
flume-ng-sinks/flume-ng-elasticsearch-sink/src/test/java/org/apache/flume/sink/elasticsearch/AbstractElasticSearchSinkTest.java 2edacdc
flume-ng-sinks/flume-ng-elasticsearch-sink/src/test/java/org/apache/flume/sink/elasticsearch/TestElasticSearchIndexRequestBuilderFactory.java PRE-CREATION
flume-ng-sinks/flume-ng-elasticsearch-sink/src/test/java/org/apache/flume/sink/elasticsearch/TestElasticSearchSink.java 94b95b1
Diff: https://reviews.apache.org/r/10835/diff/
Testing
-------
Unit tests added and run.
Patch applied and run successfully in my own flume test environment.
Thanks,
Tim Bacon