Skip to content

HPCC-35524 Add logaccess plugin load analytics #20753

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

Jump to bottom
Open
rpastrana wants to merge 2 commits into
base: candidate-9.12.x
Choose a base branch
from
Open

HPCC-35524 Add logaccess plugin load analytics #20753

rpastrana wants to merge 2 commits into from
+189 −2

Conversation

@rpastrana
Copy link
Member

@rpastrana rpastrana commented Dec 17, 2025 •
edited
Loading

  • Adds plugin/configuration issue analysis

Co-authored-by: Rodrigo Pastranarodrigo.pastrana@lexisnexis.com

Type of change:

  • This change is a bug fix (non-breaking change which fixes an issue).
  • This change is a new feature (non-breaking change which adds functionality).
  • This change improves the code (refactor or other change that does not change the functionality)
  • This change fixes warnings (the fix does not alter the functionality or the generated code)
  • This change is a breaking change (fix or feature that will cause existing behavior to change).
  • This change alters the query API (existing queries will have to be recompiled)

Checklist:

  • My code follows the code style of this project.
    • My code does not create any new warnings from compiler, build system, or lint.
  • The commit message is properly formatted and free of typos.
    • The commit message title makes sense in a changelog, by itself.
    • The commit is signed.
  • My change requires a change to the documentation.
    • I have updated the documentation accordingly, or...
    • I have created a JIRA ticket to update the documentation.
    • Any new interfaces or exported functions are appropriately commented.
  • I have read the CONTRIBUTORS document.
  • The change has been fully tested:
    • I have added tests to cover my changes.
    • All new and existing tests passed.
    • I have checked that this change does not introduce memory leaks.
    • I have used Valgrind or similar tools to check for potential issues.
  • I have given due consideration to all of the following potential concerns:
    • Scalability
    • Performance
    • Security
    • Thread-safety
    • Cloud-compatibility
    • Premature optimization
    • Existing deployed queries will not be broken
    • This change fixes the problem, not just the symptom
    • The target branch of this pull request is appropriate for such a change.
  • There are no similar instances of the same problem that should be addressed
    • I have addressed them here
    • I have raised JIRA issues to address them separately
  • This is a user interface / front-end modification
    • I have tested my changes in multiple modern browsers
    • The component(s) render as expected

Smoketest:

  • Send notifications about my Pull Request position in Smoketest queue.
  • Test my draft Pull Request.

Testing:

Copilot AI review requested due to automatic review settings December 17, 2025 21:42
@github-actions
Copy link

github-actions bot commented Dec 17, 2025

Jira Issue: https://hpccsystems.atlassian.net//browse/HPCC-35524

Jirabot Action Result:
Assigning user: rodrigo.pastrana@lexisnexisrisk.com
Workflow Transition To: Merge Pending
Updated PR

Copilot AI reviewed Dec 17, 2025
View reviewed changes
Copy link
Contributor

Copilot AI left a 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 adds enhanced diagnostics for logaccess plugin loading failures in the getHealthReport endpoint. When a logaccess plugin fails to load, the system now performs detailed diagnostics to determine whether the issue is due to missing configuration, incorrect plugin type specification, library loading failures, or factory procedure lookup problems.

Key changes:

  • Added a diagnostic function to analyze plugin loading issues independently of the normal plugin loading flow
  • Enhanced health report to provide specific, actionable error messages based on diagnostic results
  • Introduced structured diagnostic information that can be included in configuration reports

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
system/jlib/jlog.hpp Added LogAccessPluginDiagnostics struct to hold diagnostic information and declared diagnoseLogAccessPluginLoad function
system/jlib/jlog.cpp Implemented diagnoseLogAccessPluginLoad function that safely attempts to load and verify the plugin library without raising exceptions
esp/services/ws_logaccess/WsLogAccessService.cpp Modified onGetHealthReport to call diagnostic function when no plugin is loaded, providing detailed error messages based on diagnostic results

@rpastrana rpastrana force-pushed the HPCC-35524-EnhanceWsLogaccess-9.12.x branch 2 times, most recently from a70b226 to a759a4b Compare December 18, 2025 19:55
@rpastrana
Copy link
Member Author

rpastrana commented Dec 19, 2025

failed ESP regression suite check can be ignored, https://hpccsystems.atlassian.net/browse/HPCC-35534

@rpastrana
HPCC-35524 Add logaccess plugin load analytics
a82fb21 
- Adds plugin/configuration issue analysis

Co-authored-by: rpastrana <915949+rpastrana@users.noreply.github.com>
@rpastrana rpastrana force-pushed the HPCC-35524-EnhanceWsLogaccess-9.12.x branch from a759a4b to a82fb21 Compare December 19, 2025 15:04
@rpastrana rpastrana changed the title HPCC-35524 Add enhanced diagnostics for logaccess plugin loading in g… HPCC-35524 Add logaccess plugin load analytics Dec 19, 2025
timothyklemm
timothyklemm reviewed Dec 19, 2025
View reviewed changes
system/jlib/jlog.cpp Outdated
diagnostics.pluginType.set(type.str());

StringBuffer libName;
libName.append("lib").append(type.str()).append("logaccess");
Copy link
Contributor

@timothyklemm timothyklemm Dec 19, 2025

Choose a reason for hiding this comment

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

Since we're in code that could be run from a Windows build, is the "lib" prefix a valid assumption? LoadSharedObject looks for it, adding it when appropriate.

@rpastrana
HPCC-35524 Code review changes
3cb2d82 
- Replace boolean flags (configFound, loadAttempted, loadSucceeded) with single LogAccessDiagnosticState enum
- Add enum values: ConfigNotFound, ConfigFoundNoType, LoadFailed, LoadSucceeded
- Rename diagnostics.state to logAccessPluginLoadState for clarity
- Update WsLogAccessService.cpp to use clean switch-based logic instead of nested if-else
- Change diagnostics.pluginType from StringAttr to StringBuffer for direct getProp() usage
- Eliminate unnecessary temporary variables in diagnoseLogAccessPluginLoad()
- Ensure consistent methodName usage across all diagnostic messages
- Update Python test suite to match new enum-based field names
- Maintain backward compatibility while improving code maintainability

Co-authored-by: rodrigo pastrana <rodrigo.pastrana@lexisnexis.com>
timothyklemm
timothyklemm reviewed Jan 2, 2026
View reviewed changes
system/jlib/jlog.cpp
}

StringBuffer libName;
libName.append("lib").append(diagnostics.pluginType.str()).append("logaccess");
Copy link
Contributor

@timothyklemm timothyklemm Jan 2, 2026

Choose a reason for hiding this comment

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

Still not sure about the inclusion of the non-Win32 value of SharedObjectPrefix is appropriate here. Linux and MacOS builds will add this automatically in LoadSharedObject (because no extension is given). It should not be added in Win32 builds. Can this code be called in Windows builds?

Copy link
Member Author

@rpastrana rpastrana Jan 5, 2026

Choose a reason for hiding this comment

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

Thanks, Tim. It looks like you spotted an inconsistency (maybe an issue) with the logaccess plugin loading logic (https://github.com/hpcc-systems/HPCC-Platform/blob/4264c9e1f49f549e63ff9d3526036b588b5f6e6b/system/jlib/jlog.cpp#L3473C48-L3473C64). To be clear, this code isn't actually attempting to load the plugin, but rather mimic the plugin loading process to ascertain potential issues. The issue spotted should be addressed separately: https://hpccsystems.atlassian.net/browse/HPCC-35558

@rpastrana rpastrana requested a review from timothyklemm January 5, 2026 21:11
timothyklemm
timothyklemm approved these changes Jan 6, 2026
View reviewed changes
Copy link
Contributor

@timothyklemm timothyklemm left a comment

Choose a reason for hiding this comment

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

Looks ok. One nitpick would be the unnecessary return statements on lines 3554 and 3562.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@timothyklemm timothyklemm timothyklemm approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@rpastrana @timothyklemm