davsclaus commented on code in PR #28038:
URL: https://github.com/apache/camel/pull/28038#discussion_r4306094142
##########
components/camel-aws/camel-aws-cloudtrail/src/main/docs/aws-cloudtrail-component.adoc:
##########
@@ -73,6 +73,21 @@ This means you won't consume the events registered in the
CloudTrail logs stored
This is important to notice, and it must be taken into account when using this
component.
+CloudTrail can make an event visible to `LookupEvents` some minutes after its
event time, so a newer event can be
+returned first. Each poll looks up the events from the newest event time
already seen, so it does not return such a
+late event. Every `lookBackInterval` (1 minute by default) the consumer
therefore also looks up the events from
+`lookBackWindow` (10 minutes by default) before now up to the newest event
time seen, and routes those it has not
+routed yet (by event id). Events with an event time before the start of the
consumer are not routed, and an event that
+becomes visible more than `lookBackWindow` after its event time is still
missed.
+
+The polls send the same `LookupEvents` requests whatever these options. The
late-event lookup adds
Review Comment:
Nit: "whatever these options" reads oddly. Maybe "The normal polls send the
same `LookupEvents` requests regardless of these options." (the catalog copy of
the .adoc needs regenerating too).
##########
components/camel-aws/camel-aws-cloudtrail/src/main/java/org/apache/camel/component/aws/cloudtrail/CloudtrailConfiguration.java:
##########
@@ -54,6 +54,23 @@ public class CloudtrailConfiguration implements Cloneable,
AwsCommonConfiguratio
private int maxResults = 50;
@UriParam(description = "Specify an event source to select events")
private String eventSource;
+ @UriParam(defaultValue = "600000", javaType = "java.time.Duration",
+ description = "CloudTrail can make an event visible to
LookupEvents minutes after its event time (AWS"
+ + " documents typical delivery within 5 minutes of
the API call), after a newer event was"
+ + " already returned; the normal poll, which looks
up from the newest event time seen, then"
+ + " misses it. Every lookBackInterval the consumer
also looks up the events from"
+ + " lookBackWindow before now up to the newest
event time seen, and routes those it has not"
+ + " routed yet (by event id). Events before the
consumer start are not routed. Cost: each"
+ + " such lookup sends ceil(N / maxResults)
LookupEvents requests, at most one per poll, where"
+ + " N is the number of events (of the eventSource)
in that range, and none when the newest"
+ + " event time seen is older than the window. With
the defaults that is one request per"
+ + " minute while at most 50 events fall in 10
minutes. The normal polls send the same"
+ + " requests as with 0. Set to 0 to disable the
late-event lookup.")
Review Comment:
This description goes into the catalog, IDE tooltips and the options table,
and the cost arithmetic is already on the component page and in the upgrade
guide. Something shorter would read better, for example: "How far back the
consumer looks again, every lookBackInterval, for events that CloudTrail made
visible after a newer event (CloudTrail typically delivers an event within 5
minutes). Events not routed yet are routed. Set to 0 to disable." A change here
needs the configurers, JSON, DSL and catalog regenerated.
##########
components/camel-aws/camel-aws-cloudtrail/src/main/java/org/apache/camel/component/aws/cloudtrail/CloudtrailConsumer.java:
##########
@@ -52,7 +70,9 @@ protected void doStart() throws Exception {
super.doStart();
if (lastTime == null) {
// Start tailing from the consumer start time rather than
replaying CloudTrail history.
- lastTime = Instant.now();
+ lastTime = now();
+ startedAt = lastTime;
+ nextLookBackAt =
startedAt.plusMillis(getEndpoint().getConfiguration().getLookBackInterval());
Review Comment:
Nothing rejects `lookBackInterval <= 0` (when `lookBackWindow > 0`). With
that value `nextLookBackAt` is always reached, so every poll also sends a
late-event page: at the default 500 ms `delay` that is about 4 LookupEvents
requests per second, against AWS's limit of 2. Could `doStart` throw an
`IllegalArgumentException` for it, or fall back to the default?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
