allthingssecurity commented on code in PR #28038:
URL: https://github.com/apache/camel/pull/28038#discussion_r4306583209
##########
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:
Thanks, done in 579846d332b6: `doStart` now throws an
`IllegalArgumentException` for a `lookBackInterval` of 0 or less while
`lookBackWindow > 0`, and also for a negative `lookBackWindow`. With
`lookBackWindow=0` the interval is not checked, since it is not used. I chose
to fail the start rather than fall back to the default, so a typo does not go
unnoticed. New tests `lookBackIntervalOfZeroOrLessIsRejected` and
`negativeLookBackWindowIsRejected` fail without the check;
`lookBackIntervalIsNotCheckedWhenTheLookBackIsDisabled` covers the disabled
case. The camel-aws-cloudtrail tests pass (10 run, 0 failures).
_Claude Code on behalf of allthingssecurity_
##########
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:
Agreed, it was too long for a tooltip. In 579846d332b6 the `lookBackWindow`
description is now your suggested text, and `lookBackInterval` says it must be
above 0 unless `lookBackWindow` is 0. The cost arithmetic stays on the
component page and in the upgrade guide. I regenerated the component JSON, the
catalog copies and the component and endpoint DSL.
_Claude Code on behalf of allthingssecurity_
##########
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:
Changed to your wording in 579846d332b6 ("The normal polls send the same
`LookupEvents` requests regardless of these options."), and the catalog copy of
the .adoc is updated too.
_Claude Code on behalf of allthingssecurity_
--
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]
