-
Notifications
You must be signed in to change notification settings - Fork 242
feat: configurable aggregation, owner references and naming for the event recorder #3604
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
afalhambra-hivemq
wants to merge
4
commits into
operator-framework:main
Choose a base branch
from
afalhambra-hivemq:feat/event-recorder-3601
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
10de57a
feat: support a default aggregation key strategy on the event recorder
afalhambra-hivemq 31a6168
feat: support owning recorded events by the object they are about
afalhambra-hivemq 913534c
feat: support custom event naming on the event recorder
afalhambra-hivemq 9d21e48
test: cover the configured event recorder end to end
afalhambra-hivemq File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
55 changes: 55 additions & 0 deletions
55
...-framework-core/src/main/java/io/javaoperatorsdk/operator/api/event/EventKeyStrategy.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,55 @@ | ||
| /* | ||
| * Copyright Java Operator SDK Authors | ||
| * | ||
| * Licensed under the Apache License, Version 2.0 (the "License"); | ||
| * you may not use this file except in compliance with the License. | ||
| * You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
| package io.javaoperatorsdk.operator.api.event; | ||
|
|
||
| import java.util.Optional; | ||
|
|
||
| import io.fabric8.kubernetes.api.model.HasMetadata; | ||
| import io.javaoperatorsdk.operator.api.reconciler.Experimental; | ||
|
|
||
| import static io.javaoperatorsdk.operator.api.reconciler.Experimental.API_MIGHT_CHANGE; | ||
|
|
||
| /** | ||
| * Derives the default aggregation key of an event, used when the {@link EventRecord} does not set | ||
| * one explicitly. The key identifies an event among the events about the same object, so that | ||
| * repeated occurrences resolve to the same event rather than to one event each, see {@link | ||
| * EventRecord#key()}. | ||
| * | ||
| * <p>An empty result leaves the record without a default key, which keeps the message part of the | ||
| * event identity. | ||
| * | ||
| * <p>Implementations are called from concurrent reconciliations and must be thread safe. | ||
| */ | ||
| @Experimental(API_MIGHT_CHANGE) | ||
| @FunctionalInterface | ||
| public interface EventKeyStrategy { | ||
|
|
||
| Optional<String> keyFor(HasMetadata regarding, EventRecord record); | ||
|
|
||
| /** No default key: the message stays part of the event identity. */ | ||
| static EventKeyStrategy none() { | ||
| return (regarding, record) -> Optional.empty(); | ||
| } | ||
|
|
||
| /** | ||
| * Aggregates by event type and reason: all occurrences of a reason resolve to one event whose | ||
| * count grows and whose message is replaced with the latest one. The right choice for events that | ||
| * report a current state rather than individual occurrences. | ||
| */ | ||
| static EventKeyStrategy byReason() { | ||
| return (regarding, record) -> Optional.of(record.type().value() + "/" + record.reason()); | ||
| } | ||
| } |
52 changes: 52 additions & 0 deletions
52
...amework-core/src/main/java/io/javaoperatorsdk/operator/api/event/EventNamingStrategy.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,52 @@ | ||
| /* | ||
| * Copyright Java Operator SDK Authors | ||
| * | ||
| * Licensed under the Apache License, Version 2.0 (the "License"); | ||
| * you may not use this file except in compliance with the License. | ||
| * You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
| package io.javaoperatorsdk.operator.api.event; | ||
|
|
||
| import java.util.Optional; | ||
|
|
||
| import io.fabric8.kubernetes.api.model.HasMetadata; | ||
| import io.javaoperatorsdk.operator.api.reconciler.Experimental; | ||
|
|
||
| import static io.javaoperatorsdk.operator.api.reconciler.Experimental.API_MIGHT_CHANGE; | ||
|
|
||
| /** | ||
| * Names the event recorded about an object. The name is what the sink looks a recorded event up by, | ||
| * so it is also the aggregation identity: two records resolving to the same name are counted as | ||
| * occurrences of one event. A name must therefore be unique among the events it should not | ||
| * aggregate with, and stable across operator restarts and replicas. | ||
| * | ||
| * <p>A name must be a valid RFC 1123 DNS subdomain: at most 253 lowercase alphanumeric characters, | ||
| * {@code -} or {@code .}, starting and ending with an alphanumeric character. The API server | ||
| * rejects other names, and since recording is best effort, an event with an invalid name is | ||
| * dropped. Names longer than the limit are truncated. A name derived from the object and a fixed | ||
| * lowercase suffix (such as {@code <object>-status-report}) satisfies all of this by construction. | ||
| * | ||
| * <p>An empty result or a blank name falls back to the default {@code <object>.<identity hash>} | ||
| * name. | ||
| * | ||
| * <p>Implementations are called from concurrent reconciliations and must be thread safe. | ||
| */ | ||
| @Experimental(API_MIGHT_CHANGE) | ||
| @FunctionalInterface | ||
| public interface EventNamingStrategy { | ||
|
|
||
| Optional<String> nameFor(HasMetadata regarding, EventRecord record); | ||
|
|
||
| /** No custom naming: every event gets the default {@code <object>.<identity hash>} name. */ | ||
| static EventNamingStrategy none() { | ||
| return (regarding, record) -> Optional.empty(); | ||
| } | ||
| } |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🔎 Supported by static analysis
🤖 get_repo_knowledge executed:
get_repo_knowledge operator-framework/java-operator-sdk /tmp/coderabbit-repo-knowledge/operator-framework-java-operator-sdk-ba8a0318/conventionsLength of output: 3584
🏁 Script executed:
Repository: operator-framework/java-operator-sdk
Length of output: 373
🏁 Script executed:
Repository: operator-framework/java-operator-sdk
Length of output: 348
🤖 get_repo_knowledge executed:
get_repo_knowledge operator-framework/java-operator-sdk /tmp/coderabbit-repo-knowledge/operator-framework-java-operator-sdk-ba8a0318/conventionsLength of output: 3584
🌐 Web query:
operator-framework java-operator-sdk DefaultEventRecorder.java truncateToMaxNameLength💡 Result:
There is no class named DefaultEventRecorder.java within the official Java Operator SDK (JOSDK) repository [1][2]. Consequently, there is no method named truncateToMaxNameLength associated with such a class in this framework [3][4][5]. The Java Operator SDK relies on the underlying Fabric8 Kubernetes Client for many Kubernetes-specific operations, including event recording [1][6]. While users have requested a higher-level EventRecorder API within the JOSDK [7], the SDK does not provide a standard class by that name. The naming-related utility methods found in the SDK are typically focused on controller or field manager names (e.g., ensureValidName in ControllerConfiguration) rather than event recording [8]. Methods like truncateToMaxNameLength are commonly found in other Go-based Kubernetes operator projects (such as those using the operator-sdk) to ensure Kubernetes object names comply with DNS-1123 standards [9].
Citations:
🏁 Script executed:
Repository: operator-framework/java-operator-sdk
Length of output: 50392
🏁 Script executed:
Repository: operator-framework/java-operator-sdk
Length of output: 4172
Validate event names before event emission.
eventNameaccepts a non-blank record name or naming-strategy result, truncates it, and only checks that it remains non-blank. Values such asstatus-,Status, andstatus_namecan reachEventBuilder.withName(...), although Kubernetes requires event metadata names to be valid RFC 1123 DNS subdomains.recordcatches the failure fromDefaultEventSinkand suppresses it, so the event is not recorded. Validate the truncated result for both sources and use the identity-hash fallback when it is invalid. Add tests for trailing separators and invalid characters.🤖 Prompt for AI Agents