Skip to content

Commit 76f97af

Browse files
feat: support custom event naming on the event recorder
1 parent 31a6168 commit 76f97af

4 files changed

Lines changed: 229 additions & 11 deletions

File tree

‎operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/event/DefaultEventRecorder.java‎

Lines changed: 57 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -48,10 +48,11 @@
4848
* can be overridden, see {@link
4949
* io.javaoperatorsdk.operator.api.config.ConfigurationService#clusterScopedEventNamespace()}.
5050
*
51-
* <p>Events are named deterministically, after the object they are about plus a hash of everything
52-
* that identifies the event, so that recording the same event again resolves to the event already
53-
* recorded for it. Repeat occurrences are then counted on that event rather than recorded as copies
54-
* of it, see {@link DefaultEventSink}.
51+
* <p>By default, events are named deterministically, after the object they are about plus a hash of
52+
* everything that identifies the event, so that recording the same event again resolves to the
53+
* event already recorded for it. Repeat occurrences are then counted on that event rather than
54+
* recorded as copies of it, see {@link DefaultEventSink}. How events aggregate, how they are named
55+
* and whether they carry an owner reference can be configured, see {@link #builder(EventSink)}.
5556
*/
5657
public class DefaultEventRecorder implements EventRecorder {
5758

@@ -74,16 +75,21 @@ public class DefaultEventRecorder implements EventRecorder {
7475
private static final int IDENTITY_HASH_LENGTH = 32;
7576

7677
private final EventSink sink;
78+
private final EventNamingStrategy namingStrategy;
7779
private final EventKeyStrategy keyStrategy;
7880
private final boolean ownerReference;
7981

8082
public DefaultEventRecorder(EventSink sink) {
81-
this(sink, EventKeyStrategy.none(), false);
83+
this(sink, EventNamingStrategy.none(), EventKeyStrategy.none(), false);
8284
}
8385

8486
private DefaultEventRecorder(
85-
EventSink sink, EventKeyStrategy keyStrategy, boolean ownerReference) {
87+
EventSink sink,
88+
EventNamingStrategy namingStrategy,
89+
EventKeyStrategy keyStrategy,
90+
boolean ownerReference) {
8691
this.sink = sink;
92+
this.namingStrategy = namingStrategy;
8793
this.keyStrategy = keyStrategy;
8894
this.ownerReference = ownerReference;
8995
}
@@ -96,16 +102,25 @@ public static Builder builder(EventSink sink) {
96102
public static final class Builder {
97103

98104
private final EventSink sink;
105+
private EventNamingStrategy namingStrategy = EventNamingStrategy.none();
99106
private EventKeyStrategy keyStrategy = EventKeyStrategy.none();
100107
private boolean ownerReference = false;
101108

102109
private Builder(EventSink sink) {
103110
this.sink = Objects.requireNonNull(sink, "sink must not be null");
104111
}
105112

113+
/** The strategy naming recorded events, see {@link EventNamingStrategy}. */
114+
public Builder namingStrategy(EventNamingStrategy namingStrategy) {
115+
this.namingStrategy =
116+
Objects.requireNonNull(namingStrategy, "namingStrategy must not be null");
117+
return this;
118+
}
119+
106120
/**
107121
* The strategy deriving the default aggregation key of records that do not set one, see {@link
108-
* EventKeyStrategy}.
122+
* EventKeyStrategy}. The key is ignored for events whose name is set by the record or resolved
123+
* by the naming strategy, see {@link EventNamingStrategy}.
109124
*/
110125
public Builder keyStrategy(EventKeyStrategy keyStrategy) {
111126
this.keyStrategy = Objects.requireNonNull(keyStrategy, "keyStrategy must not be null");
@@ -126,7 +141,7 @@ public Builder ownerReference(boolean ownerReference) {
126141
}
127142

128143
public DefaultEventRecorder build() {
129-
return new DefaultEventRecorder(sink, keyStrategy, ownerReference);
144+
return new DefaultEventRecorder(sink, namingStrategy, keyStrategy, ownerReference);
130145
}
131146
}
132147

@@ -162,14 +177,17 @@ private static String resolve() {
162177
public void record(EventRecord event, Context<?> context) {
163178
Objects.requireNonNull(context, "the context of the reconciliation must not be null");
164179
Objects.requireNonNull(event, "event must not be null");
180+
Event assembled = null;
165181
try {
166-
sink.emit(toEvent(context, event), context);
182+
assembled = toEvent(context, event);
183+
sink.emit(assembled, context);
167184
} catch (Exception e) {
168185
// recording an event must never break the caller: a controller that fails to reconcile
169186
// because it could not write an event is strictly worse than one that records nothing
170187
log.warn(
171-
"Could not record {} event with reason {} for resource {} in namespace {}",
188+
"Could not record {} event named {} with reason {} for resource {} in namespace {}",
172189
event.type(),
190+
assembled != null ? assembled.getMetadata().getName() : "unknown",
173191
event.reason(),
174192
context.getPrimaryResource().getMetadata().getName(),
175193
context.getPrimaryResource().getMetadata().getNamespace(),
@@ -249,6 +267,30 @@ private String eventNamespace(HasMetadata regarding, Context<?> context) {
249267
CLUSTER_SCOPED_EVENT_NAMESPACE);
250268
}
251269

270+
private String eventName(HasMetadata regarding, EventRecord record, String reportingController) {
271+
return record
272+
.name()
273+
.filter(name -> !name.isBlank())
274+
.or(() -> namingStrategy.nameFor(regarding, record).filter(name -> !name.isBlank()))
275+
.map(DefaultEventRecorder::truncateToMaxNameLength)
276+
.orElseGet(() -> identityHashName(regarding, record, reportingController));
277+
}
278+
279+
private static String truncateToMaxNameLength(String name) {
280+
if (name.length() <= MAX_NAME_LENGTH) {
281+
return name;
282+
}
283+
var truncated = name.substring(0, MAX_NAME_LENGTH);
284+
while (truncated.endsWith("-") || truncated.endsWith(".")) {
285+
truncated = truncated.substring(0, truncated.length() - 1);
286+
}
287+
log.warn(
288+
"Truncated the name of event {} to {} to stay within the Kubernetes name limit",
289+
name,
290+
truncated);
291+
return truncated;
292+
}
293+
252294
/**
253295
* Names events {@code <object name>.<hash>}, following the convention of the Go client, hashing
254296
* everything that makes two events the same event: the object, the type, the reason, the
@@ -259,8 +301,12 @@ private String eventNamespace(HasMetadata regarding, Context<?> context) {
259301
*
260302
* <p>The object is identified by its uid, with the kind as a fallback for objects that do not
261303
* have one yet, such as a dependent resource that has only been built so far.
304+
*
305+
* <p>This is the fallback when the record does not set a name and the naming strategy resolves to
306+
* nothing, see {@link EventNamingStrategy}.
262307
*/
263-
private String eventName(HasMetadata regarding, EventRecord record, String reportingController) {
308+
private String identityHashName(
309+
HasMetadata regarding, EventRecord record, String reportingController) {
264310
var metadata = regarding.getMetadata();
265311
var identity =
266312
String.join(
Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
/*
2+
* Copyright Java Operator SDK Authors
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package io.javaoperatorsdk.operator.api.event;
17+
18+
import java.util.Optional;
19+
20+
import io.fabric8.kubernetes.api.model.HasMetadata;
21+
import io.javaoperatorsdk.operator.api.reconciler.Experimental;
22+
23+
import static io.javaoperatorsdk.operator.api.reconciler.Experimental.API_MIGHT_CHANGE;
24+
25+
/**
26+
* Names the event recorded about an object. The name is what the sink looks a recorded event up by,
27+
* so it is also the aggregation identity: two records resolving to the same name are counted as
28+
* occurrences of one event. A name must therefore be unique among the events it should not
29+
* aggregate with, and stable across operator restarts and replicas.
30+
*
31+
* <p>A name must be a valid RFC 1123 DNS subdomain: at most 253 lowercase alphanumeric characters,
32+
* {@code -} or {@code .}, starting and ending with an alphanumeric character. The API server
33+
* rejects other names, and since recording is best effort, an event with an invalid name is
34+
* dropped. Names longer than the limit are truncated. A name derived from the object and a fixed
35+
* lowercase suffix (such as {@code <object>-status-report}) satisfies all of this by construction.
36+
*
37+
* <p>An empty result or a blank name falls back to the default {@code <object>.<identity hash>}
38+
* name.
39+
*
40+
* <p>Implementations are called from concurrent reconciliations and must be thread safe.
41+
*/
42+
@Experimental(API_MIGHT_CHANGE)
43+
@FunctionalInterface
44+
public interface EventNamingStrategy {
45+
46+
Optional<String> nameFor(HasMetadata regarding, EventRecord record);
47+
48+
/** No custom naming: every event gets the default {@code <object>.<identity hash>} name. */
49+
static EventNamingStrategy none() {
50+
return (regarding, record) -> Optional.empty();
51+
}
52+
}

‎operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/event/EventRecord.java‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ public final class EventRecord {
3232
private final EventType type;
3333
private final String reason;
3434
private final String message;
35+
private final String name;
3536
private final String key;
3637
private final Boolean ownedByRegarding;
3738
private final String action;
@@ -43,6 +44,7 @@ private EventRecord(Builder builder) {
4344
this.type = builder.type;
4445
this.reason = builder.reason;
4546
this.message = builder.message;
47+
this.name = builder.name;
4648
this.key = builder.key;
4749
this.action = builder.action;
4850
this.reportingComponent = builder.reportingComponent;
@@ -77,9 +79,21 @@ public String message() {
7779
return message;
7880
}
7981

82+
/**
83+
* The name of the recorded event, overriding the recorder's naming. The name is the aggregation
84+
* identity and must be a valid RFC 1123 DNS subdomain, see {@link EventNamingStrategy}. A blank
85+
* name is treated as unset.
86+
*/
87+
public Optional<String> name() {
88+
return Optional.ofNullable(name);
89+
}
90+
8091
/**
8192
* Identifies this event among the events about the same object, so that repeated occurrences
8293
* resolve to the same event rather than to one event each.
94+
*
95+
* <p>The key is ignored when the record sets a {@link #name()} or the recorder's naming strategy
96+
* resolves one: the name is then the aggregation identity on its own.
8397
*/
8498
public Optional<String> key() {
8599
return Optional.ofNullable(key);
@@ -129,6 +143,7 @@ public static final class Builder {
129143
private EventType type = EventType.NORMAL;
130144
private String reason;
131145
private String message;
146+
private String name;
132147
private String key;
133148
private Boolean ownedByRegarding;
134149
private String action;
@@ -153,6 +168,12 @@ public Builder message(String message) {
153168
return this;
154169
}
155170

171+
/** Sets the name of the recorded event, see {@link EventRecord#name()}. */
172+
public Builder name(String name) {
173+
this.name = name;
174+
return this;
175+
}
176+
156177
/** Sets the key identifying this event, see {@link EventRecord#key()}. */
157178
public Builder key(String key) {
158179
this.key = key;

‎operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/event/DefaultEventRecorderTest.java‎

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -382,6 +382,105 @@ void setsNoOwnerReferenceWhenTheRegardingObjectHasNoUidYet() {
382382
assertThat(emitted.get(0).getMetadata().getOwnerReferences()).isEmpty();
383383
}
384384

385+
@Test
386+
void namesTheEventAfterThePerRecordNameWhenOneIsSet() {
387+
recorder.record(
388+
EventRecord.builder().reason("Created").message("created").name("my-event-name").build(),
389+
context(configMap()));
390+
391+
assertThat(emitted.get(0).getMetadata().getName()).isEqualTo("my-event-name");
392+
}
393+
394+
@Test
395+
void namesEventsThroughTheNamingStrategy() {
396+
var recorder =
397+
DefaultEventRecorder.builder((event, context) -> emitted.add(event))
398+
.namingStrategy(
399+
(regarding, record) ->
400+
Optional.of(regarding.getMetadata().getName() + "-" + record.reason()))
401+
.build();
402+
403+
recorder.record(EventRecord.warning("failed", "first"), context(configMap()));
404+
recorder.record(EventRecord.warning("failed", "second"), context(configMap()));
405+
406+
assertThat(emitted)
407+
.allSatisfy(event -> assertThat(event.getMetadata().getName()).isEqualTo("test1-failed"));
408+
}
409+
410+
@Test
411+
void fallsBackToTheIdentityHashNameWhenTheNamingStrategyResolvesNothing() {
412+
var recorder =
413+
DefaultEventRecorder.builder((event, context) -> emitted.add(event))
414+
.namingStrategy((regarding, record) -> Optional.empty())
415+
.build();
416+
417+
recorder.record(EventRecord.warning("Failed", "message"), context(configMap()));
418+
419+
assertThat(emitted.get(0).getMetadata().getName())
420+
.startsWith("test1.")
421+
.hasSize("test1.".length() + 32);
422+
}
423+
424+
@Test
425+
void truncatesSuppliedNamesToTheKubernetesNameLengthLimit() {
426+
recorder.record(
427+
EventRecord.builder().reason("Created").message("created").name("a".repeat(300)).build(),
428+
context(configMap()));
429+
430+
assertThat(emitted.get(0).getMetadata().getName()).hasSize(253);
431+
}
432+
433+
@Test
434+
void stripsTrailingSeparatorsFromTruncatedNames() {
435+
recorder.record(
436+
EventRecord.builder()
437+
.reason("Created")
438+
.message("created")
439+
.name("a".repeat(252) + "." + "b".repeat(47))
440+
.build(),
441+
context(configMap()));
442+
recorder.record(
443+
EventRecord.builder()
444+
.reason("Created")
445+
.message("created")
446+
.name("b".repeat(252) + "-" + "c".repeat(47))
447+
.build(),
448+
context(configMap()));
449+
450+
assertThat(emitted.get(0).getMetadata().getName()).isEqualTo("a".repeat(252));
451+
assertThat(emitted.get(1).getMetadata().getName()).isEqualTo("b".repeat(252));
452+
}
453+
454+
@Test
455+
void treatsABlankNameAsUnset() {
456+
var recorder =
457+
DefaultEventRecorder.builder((event, context) -> emitted.add(event))
458+
.namingStrategy((regarding, record) -> Optional.of(" "))
459+
.build();
460+
461+
recorder.record(
462+
EventRecord.builder().reason("Created").message("created").name("").build(),
463+
context(configMap()));
464+
465+
assertThat(emitted.get(0).getMetadata().getName()).matches("test1\\.[0-9a-f]{32}");
466+
}
467+
468+
@Test
469+
void aFailingNamingStrategyNeverFailsTheCaller() {
470+
var recorder =
471+
DefaultEventRecorder.builder((event, context) -> emitted.add(event))
472+
.namingStrategy(
473+
(regarding, record) -> {
474+
throw new RuntimeException("cannot derive a name");
475+
})
476+
.build();
477+
478+
assertThatCode(
479+
() -> recorder.record(EventRecord.normal("Created", "created"), context(configMap())))
480+
.doesNotThrowAnyException();
481+
assertThat(emitted).isEmpty();
482+
}
483+
385484
Context<?> context(HasMetadata primaryResource) {
386485
return context(primaryResource, DefaultEventRecorder.CLUSTER_SCOPED_EVENT_NAMESPACE);
387486
}

0 commit comments

Comments
 (0)