Skip to content

Filter OSS Packets#2370

Merged
jhiemstrawisc merged 6 commits into
PelicanPlatform:mainfrom
patrickbrophy:filter-oss-packets
Jun 13, 2025
Merged

Filter OSS Packets#2370
jhiemstrawisc merged 6 commits into
PelicanPlatform:mainfrom
patrickbrophy:filter-oss-packets

Conversation

@patrickbrophy

Copy link
Copy Markdown
Contributor

This PR adds an allow list to the OSS packet handler so that we can filter packets that are disallowed from being processed into metrics. If we want to add another event we can just update the allow list.

@patrickbrophy patrickbrophy added the bug Something isn't working label May 30, 2025

@bbockelm bbockelm left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This only tests if the last event in the list is an expected OSS event type.

If the server timing is such that the unknown event is always last, then the desired OSS event will always be ignored.

Probably need to change things so you iterate backward through the packets.

Comment thread metrics/xrootd_metrics.go Outdated
Comment thread metrics/xrootd_metrics.go Outdated
Comment thread metrics/xrootd_metrics_test.go Outdated
Comment thread metrics/xrootd_metrics_test.go Outdated
@h2zh h2zh added the create-patch Patch this into multiple versions of Pelican label Jun 2, 2025
@patrickbrophy patrickbrophy requested a review from bbockelm June 3, 2025 16:28
Comment thread metrics/xrootd_metrics.go
Comment thread metrics/xrootd_metrics.go Outdated
Comment thread metrics/xrootd_metrics.go Outdated
Comment thread metrics/xrootd_metrics_test.go Outdated
Comment thread metrics/xrootd_metrics_test.go Outdated
Comment thread metrics/xrootd_metrics_test.go
Comment thread metrics/xrootd_metrics_test.go Outdated
Comment thread metrics/xrootd_metrics.go
@jhiemstrawisc jhiemstrawisc added cache Issue relating to the cache component origin Issue relating to the origin component labels Jun 13, 2025

@jhiemstrawisc jhiemstrawisc left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Changes look good to me, thanks!

Let's double check that the currently-failing test is just a fluke (looks unrelated, but I'd like to double check).

Before you merge, can you fix the "PR Validation" test by creating/linking an issue that describes what symptoms this extra handling was solving?

@patrickbrophy patrickbrophy linked an issue Jun 13, 2025 that may be closed by this pull request
@jhiemstrawisc

Copy link
Copy Markdown
Member

Using super powers to merge this -- I believe Patrick has addressed BrianB's comments.

@jhiemstrawisc jhiemstrawisc merged commit 2dcb061 into PelicanPlatform:main Jun 13, 2025
27 of 31 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working cache Issue relating to the cache component create-patch Patch this into multiple versions of Pelican origin Issue relating to the origin component

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Filter XRootD OSS Packets

4 participants