fix(stats): Parse Event objects into structured POJOs for Telemetry events #660
Conversation
This file contains 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
ezimanyi
reviewed
Sep 24, 2019
echo-telemetry/src/main/java/com/netflix/spinnaker/echo/config/TelemetryConfig.java
Show resolved
Hide resolved
echo-telemetry/src/main/java/com/netflix/spinnaker/echo/telemetry/TelemetryEventListener.java
Outdated
Show resolved
Hide resolved
echo-telemetry/src/main/java/com/netflix/spinnaker/echo/telemetry/TelemetryEventListener.java
Outdated
Show resolved
Hide resolved
echo-telemetry/src/main/java/com/netflix/spinnaker/echo/telemetry/TelemetryEventListener.java
Show resolved
Hide resolved
echo-telemetry/src/main/java/com/netflix/spinnaker/echo/telemetry/TelemetryEventListener.java
Outdated
Show resolved
Hide resolved
echo-telemetry/src/main/java/com/netflix/spinnaker/echo/telemetry/TelemetryEventListener.java
Outdated
Show resolved
Hide resolved
echo-telemetry/src/main/java/com/netflix/spinnaker/echo/telemetry/TelemetryEventListener.java
Outdated
Show resolved
Hide resolved
ezimanyi
approved these changes
Sep 25, 2019
| public static class Stage { | ||
| @Builder.Default private String status = "UNKNOWN"; | ||
| @Builder.Default private String type = "UNKNOWN"; | ||
| @Builder.Default Context context = Context.builder().build(); |
There was a problem hiding this comment.
This (and the next var) can/should be private.
| @Builder | ||
| @JsonDeserialize(builder = Content.ContentBuilder.class) | ||
| public static class Content { | ||
| @Builder.Default private Execution execution = Execution.builder().build(); |
There was a problem hiding this comment.
Now that these classes are immutable 🎉 🎉 , all the member variables can/should be made final.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Fixes spinnaker/spinnaker#4885
I tried (a lot) to make this broader - to change Event.content from a
Mapto a proper POJOContentobject - but I couldn't get Jackson deserialization to work as expected and not have to change large amounts of both prod and test code (access likeevent.content.something.somethingElseis pervasive across this codebase).So instead I opted to fix it locally - grabbing the Map and converting it locally to known fields and setting reasonable defaults for those fields, should they be absent.
One thing to note also is the wrapper
TELobject - because there are a lot of duplicate names here, I wrapped the local objects in a wrapper so I wouldn't have to use long, fully qualified names everywhere.