Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Allow @ in remote user format string when parsing web server log files. #1816

Conversation

aaronweeden
Copy link
Contributor

Description

This PR updates the web server log file parser to allow @ in remote user values.

Motivation and Context

Remote user values are logged by some identity providers in the form <user>@<domain>. This PR makes sure that the log parser does not skip these lines. This is relevant for the ondemand module and was noticed when testing the upcoming OnDemand realm in ACCESS XDMoD.

Tests performed

I tested this by running xdmod-ondemand-ingestor on my port on xdmod-dev with log files containing user values containing @. Before the changes in this PR, the lines are skipped; after, they are ingested.

Checklist:

  • The pull request description is suitable for a Changelog entry
  • The milestone is set correctly on the pull request
  • The appropriate labels have been added to the pull request

@aaronweeden aaronweeden added the enhancement Enhancement of the functionality of an existing feature label Feb 2, 2024
@aaronweeden aaronweeden added this to the 11.0.0 milestone Feb 2, 2024
@aaronweeden aaronweeden force-pushed the allow-at-sign-in-remote-user-format-string branch from 69b337b to b646ca6 Compare February 2, 2024 22:15
@aaronweeden aaronweeden changed the title Allow "at" # remote user format string when parsing web server log files. Allow @ in remote user format string when parsing web server log files. Feb 6, 2024
@aaronweeden aaronweeden force-pushed the allow-at-sign-in-remote-user-format-string branch from b646ca6 to d60b543 Compare February 6, 2024 14:21
@jpwhite4
Copy link
Member

jpwhite4 commented Feb 8, 2024

It seem this change will not be needed with the next (as yet unreleased) version of LogParser. To help us keep the code clean in future I suggest that you add a comment to the composer.json next to the LogParser dependency metadata that reminds us to remove the remote user format string override if we change the version number. (I think you could use the "description" property to do this).

@aaronweeden aaronweeden force-pushed the allow-at-sign-in-remote-user-format-string branch 5 times, most recently from 2680b6a to 3bff18e Compare February 8, 2024 21:34
@aaronweeden
Copy link
Contributor Author

@jpwhite4 I have added a comment above the code and a comment to composer.json using "extra". I have also set myself up for GitHub notifications of new releases from kassner/log-parser.

@aaronweeden aaronweeden force-pushed the allow-at-sign-in-remote-user-format-string branch from 3bff18e to 3c8ea5e Compare February 20, 2024 16:43
@aaronweeden aaronweeden force-pushed the allow-at-sign-in-remote-user-format-string branch from 3c8ea5e to 38f6d9e Compare February 20, 2024 17:04
@aaronweeden aaronweeden merged commit e5f3bee into ubccr:xdmod11.0 Feb 20, 2024
4 checks passed
@aaronweeden aaronweeden deleted the allow-at-sign-in-remote-user-format-string branch February 20, 2024 17:56
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
enhancement Enhancement of the functionality of an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants