-
Notifications
You must be signed in to change notification settings - Fork 3.1k
Fix journal view to display updated application names after registration changes #4554
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
Conversation
@ulischulte 👋 This repository doesn't have Copilot instructions. With Copilot instructions, I can understand the repository better, work faster and produce higher quality PRs. I can generate a .github/copilot-instructions.md file for you automatically. Click here to open a pre-filled issue and assign it to me. I'll write the instructions, and then tag you for review. |
spring-boot-admin-server-ui/src/main/frontend/views/journal/index.vue
Outdated
Show resolved
Hide resolved
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.
Pull Request Overview
This PR fixes a bug where the journal view displayed outdated application names after applications updated their registration information. The root cause was that the journal only considered REGISTERED
events when building instance name mappings, ignoring REGISTRATION_UPDATED
events that contain updated names.
- Updated the event filter to include both
REGISTERED
andREGISTRATION_UPDATED
events - Modified the reduce operation to process events chronologically so the most recent name is used
- Added comprehensive test coverage for the fix
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
spring-boot-admin-server-ui/src/main/frontend/views/journal/index.vue | Updated instanceNames computed property to filter for both registration event types and process them chronologically |
spring-boot-admin-server-ui/src/main/frontend/views/journal/index.spec.ts | Added new test file with comprehensive coverage for the registration update functionality |
}); | ||
|
||
const instanceNames = | ||
container.firstChild.__vueParentComponent.ctx.instanceNames; |
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.
in other tests we do not use properties, but check what is visible on screen, right?
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.
maybe @SteKoe should review this before merge
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.
Thanks. The tests now are using screen
to check for the instance names.
…ion changes Co-authored-by: ulischulte <[email protected]>
…dex.vue Co-authored-by: Copilot <[email protected]>
cf8b541
to
c5051af
Compare
The journal view was displaying outdated application names even after applications updated their registration information with new names. This occurred because the journal only considered
REGISTERED
events when building the instance name mapping, completely ignoringREGISTRATION_UPDATED
events that contain the updated application names.Problem
When an application changes its name and sends a
REGISTRATION_UPDATED
event to Spring Boot Admin, the journal view continued to show the old name from the originalREGISTERED
event. This created confusion as:Root Cause
The
instanceNames
computed property in/views/journal/index.vue
only filtered forInstanceEvent.REGISTERED
events:This meant that subsequent
REGISTRATION_UPDATED
events were ignored when building the instance name lookup map.Solution
Updated the filter to include both registration event types:
Since events are processed in chronological order and later events overwrite earlier ones in the reduce operation, the most recent application name will always be used, whether from an initial registration or a subsequent update.
Testing
The fix is minimal and surgical, changing only the event filtering logic without affecting any other journal functionality.
Fixes #4476.
Warning
Firewall rules blocked me from connecting to one or more addresses
I tried to connect to the following addresses, but was blocked by firewall rules:
artifactory-oss.prod.netflix.net
/usr/lib/jvm/temurin-17-jdk-amd64/bin/java --enable-native-access=ALL-UNNAMED -classpath /home/REDACTED/.m2/wrapper/dists/apache-maven-3.9.11/a2d47e15/boot/plexus-classworlds-2.9.0.jar -Dclassworlds.conf=/home/REDACTED/.m2/wrapper/dists/apache-maven-3.9.11/a2d47e15/bin/m2.conf -Dmaven.home=/home/REDACTED/.m2/wrapper/dists/apache-maven-3.9.11/a2d47e15 -Dlibrary.jansi.path=/home/REDACTED/.m2/wrapper/dists/apache-maven-3.9.11/a2d47e15/lib/jansi-native -Dmaven.multiModuleProjectDirectory=/home/REDACTED/work/spring-boot-admin/spring-boot-admin org.codehaus.plexus.classworlds.launcher.Launcher clean compile -DskipTests -q
(dns block)maven.repository.redhat.com
/usr/lib/jvm/temurin-17-jdk-amd64/bin/java --enable-native-access=ALL-UNNAMED -classpath /home/REDACTED/.m2/wrapper/dists/apache-maven-3.9.11/a2d47e15/boot/plexus-classworlds-2.9.0.jar -Dclassworlds.conf=/home/REDACTED/.m2/wrapper/dists/apache-maven-3.9.11/a2d47e15/bin/m2.conf -Dmaven.home=/home/REDACTED/.m2/wrapper/dists/apache-maven-3.9.11/a2d47e15 -Dlibrary.jansi.path=/home/REDACTED/.m2/wrapper/dists/apache-maven-3.9.11/a2d47e15/lib/jansi-native -Dmaven.multiModuleProjectDirectory=/home/REDACTED/work/spring-boot-admin/spring-boot-admin org.codehaus.plexus.classworlds.launcher.Launcher clean compile -DskipTests -q
(dns block)packages.confluent.io
/usr/lib/jvm/temurin-17-jdk-amd64/bin/java --enable-native-access=ALL-UNNAMED -classpath /home/REDACTED/.m2/wrapper/dists/apache-maven-3.9.11/a2d47e15/boot/plexus-classworlds-2.9.0.jar -Dclassworlds.conf=/home/REDACTED/.m2/wrapper/dists/apache-maven-3.9.11/a2d47e15/bin/m2.conf -Dmaven.home=/home/REDACTED/.m2/wrapper/dists/apache-maven-3.9.11/a2d47e15 -Dlibrary.jansi.path=/home/REDACTED/.m2/wrapper/dists/apache-maven-3.9.11/a2d47e15/lib/jansi-native -Dmaven.multiModuleProjectDirectory=/home/REDACTED/work/spring-boot-admin/spring-boot-admin org.codehaus.plexus.classworlds.launcher.Launcher clean compile -DskipTests -q
(dns block)repository.hazelcast.com
/usr/lib/jvm/temurin-17-jdk-amd64/bin/java --enable-native-access=ALL-UNNAMED -classpath /home/REDACTED/.m2/wrapper/dists/apache-maven-3.9.11/a2d47e15/boot/plexus-classworlds-2.9.0.jar -Dclassworlds.conf=/home/REDACTED/.m2/wrapper/dists/apache-maven-3.9.11/a2d47e15/bin/m2.conf -Dmaven.home=/home/REDACTED/.m2/wrapper/dists/apache-maven-3.9.11/a2d47e15 -Dlibrary.jansi.path=/home/REDACTED/.m2/wrapper/dists/apache-maven-3.9.11/a2d47e15/lib/jansi-native -Dmaven.multiModuleProjectDirectory=/home/REDACTED/work/spring-boot-admin/spring-boot-admin org.codehaus.plexus.classworlds.launcher.Launcher clean compile -DskipTests -q
(dns block)If you need me to access, download, or install something from one of these locations, you can either:
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.