Skip to content

fix(#4858): deduplicate events #4872

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.

Sign up for GitHub

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

Merged
SteKoe merged 7 commits into from
Feb 10, 2026
Merged

fix(#4858): deduplicate events #4872

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
e72ebf4
fix-4858 dedeuplicate
Nov 24, 2025
b6f7bf2
removing comments
Nov 24, 2025
fb949dd
Merge branch 'master' into fix-4858-deduplicate
teja2 Nov 24, 2025
0cc0842
Merge branch 'master' into fix-4858-deduplicate
SteKoe Dec 12, 2025
d86b76a
fix: enhance deduplication logic for instance events to include type …
SteKoe Feb 10, 2026
f892412
Merge branch 'master' into fix-4858-deduplicate
SteKoe Feb 10, 2026
6215c2d
fix: reorder imports for deduplication logic in index.vue
SteKoe Feb 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 52 additions & 0 deletions spring-boot-admin-server-ui/src/main/frontend/views/journal/deduplicate-events.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
import { describe, expect, it } from 'vitest';

import { InstanceEvent } from '@/views/journal/InstanceEvent';

import { deduplicateInstanceEvents } from './deduplicate-events';

const createEvent = (instance: string, version: number) =>
new InstanceEvent({
instance,
version,
type: 'REGISTERED',
timestamp: '2024-01-01T10:00:00Z',
registration: { name: instance },
});

describe('deduplicateInstanceEvents', () => {
it('removes events with identical instance and version', () => {
const events = [
createEvent('instance-1', 1),
createEvent('instance-1', 1),
createEvent('instance-2', 3),
createEvent('instance-2', 3),
createEvent('instance-3', 2),
];

const result = deduplicateInstanceEvents(events);

expect(result).toHaveLength(3);
expect(result.map((event) => event.key)).toEqual([
'instance-1-1',
'instance-2-3',
'instance-3-2',
]);
});

it('preserves the order of the first occurrences', () => {
const events = [
createEvent('instance-1', 2),
createEvent('instance-2', 1),
createEvent('instance-1', 2),
createEvent('instance-3', 4),
];

const result = deduplicateInstanceEvents(events);

expect(result.map((event) => event.key)).toEqual([
'instance-1-2',
'instance-2-1',
'instance-3-4',
]);
});
});
12 changes: 12 additions & 0 deletions spring-boot-admin-server-ui/src/main/frontend/views/journal/deduplicate-events.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import { InstanceEvent } from '@/views/journal/InstanceEvent';

export function deduplicateInstanceEvents(events: InstanceEvent[]) {
const seen = new Set<string>();
return events.filter((event) => {
if (seen.has(event.key)) {
Copy link
Copy Markdown
Contributor

@cdprete cdprete Feb 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SteKoe is the event key really enough?
It's imperative, in my opinion, that a SBA instance X shows the events recorded by the same instance and not, for example, by the instance Y.
Otherwise there is no traceability anymore and it's, therefore, better to keep things as they're now by enduring the duplicates.

Copy link
Copy Markdown
Contributor

@SteKoe SteKoe Feb 10, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the backend does not contain the instance name of SBA in the events, this would be a bigger change than just adding deduplication in frontend.

As key is just using instanceId and version, no. We have to respect any value here. Event type as well as timestamp. Otherwise the following events would be reduced, even though they are not the same:

const events = [
  {instance: '123', version: 1, type: "REGISTERED", timestamp: "678"},
  {instance: '123', version: 1, type: "DEREGISTERED", timestamp: "890"},
]

Thanks to the existing tests, I was able to extend the behavior and added some more use cases.

Copy link
Copy Markdown
Contributor

@cdprete cdprete Feb 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But then you won't dedup anything I think

return false;
}
seen.add(event.key);
return true;
});
}
18 changes: 12 additions & 6 deletions spring-boot-admin-server-ui/src/main/frontend/views/journal/index.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,9 @@ import { useDateTimeFormatter } from '@/composables/useDateTimeFormatter';
import subscribing from '@/mixins/subscribing';
import Instance from '@/services/instance';
import { compareBy } from '@/utils/collections';
import { InstanceEvent } from '@/views/journal/InstanceEvent';
import { deduplicateInstanceEvents } from '@/views/journal/deduplicate-events';
import {
InstanceEvent,
InstanceEventType,
} from '@/views/journal/InstanceEvent';
import JournalTable from '@/views/journal/JournalTable.vue';
Expand All @@ -67,6 +68,7 @@ export default {
data: () => ({
Event,
events: [],
seenEventKeys: new Set(),
listOffset: 0,
showPayload: {},
pageSize: 25,
Expand Down Expand Up @@ -104,7 +106,9 @@ export default {
.reverse()
.map((e) => new InstanceEvent(e));

this.events = Object.freeze(events);
const deduplicated = deduplicateInstanceEvents(events);
this.seenEventKeys = new Set(deduplicated.map((event) => event.key));
this.events = Object.freeze(deduplicated);
this.error = null;
} catch (error) {
console.warn('Fetching events failed:', error);
Expand All @@ -119,10 +123,12 @@ export default {
return Instance.getEventStream().subscribe({
next: (message) => {
this.error = null;
this.events = Object.freeze([
new InstanceEvent(message.data),
...this.events,
]);
const incomingEvent = new InstanceEvent(message.data);
if (this.seenEventKeys.has(incomingEvent.key)) {
return;
Copy link
Copy Markdown
Contributor

@cdprete cdprete Nov 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this may be wrong here because then listOffset doesn't get updated properly.
@SteKoe @ulischulte your thoughts on this?

Copy link
Copy Markdown
Contributor

@SteKoe SteKoe Dec 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right, but as far as I can see, the offset is not used in the view anyway. I will dig into that and check if it is still needed.

Copy link
Copy Markdown
Contributor

@cdprete cdprete Feb 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SteKoe any updates on this PR?

Copy link
Copy Markdown
Contributor

@SteKoe SteKoe Feb 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

listOffset is now respected and adjusted.

}
this.seenEventKeys.add(incomingEvent.key);
this.events = Object.freeze([incomingEvent, ...this.events]);
this.listOffset += 1;
},
error: (error) => {
Expand Down