Reduce noise from NotEntitledException logging #124511
Conversation
Some of these are expected, so an INFO seems more appropriate. The stack trace tends to attract attention even when entitlements are not the cause of a problem, so let's avoid the stack trace, but still include stack frame info from the frame of interest.
prdoyle
added
>non-issue
auto-backport
|
Pinging @elastic/es-core-infra (Team:Core/Infra) |
Instead of making the unexpected ones look "ok" with info level, can we hide the expected ones? eg can we create a child logger based on the module name + entitlement (or some other identifier) that we can mute with normal logging filtering? Then GCS/AWS libs can ignore those known warnings without flooding the ES log, but expected entitlement failures can be properly warned. |
If we filter out the "known" entitlements failures we want to ignore, is that still the case? We wanted to ensure the log contains everything we need to fix the issue, even if the exception is caught and swallowed. |
Sorry @rjernst my mistake, I totally misread what you said as "instead of making the expected ones look ok". |
That's not what I was suggesting. Rather, I think we should use a unique logger for each module, so that we can control changing the log level per plugin/module. See for example We could conceivably do something more complicated (eg include the entitlement type in the logger name as well), but module name is probably sufficient for now. |
|
Ok I'm not sure I see the advantage of that approach. Emitting warnings when everything is operating as intended seems bad. Perhaps I'll understand better after our sync meeting today. |
|
Thinking about it some more, I like the "child logger" idea irrespective of |
|
Note to self: if the module has no name, we could use the same |
| // Don't emit a log for muted classes, e.g. classes containing self tests | ||
| if (mutedClasses.contains(callerClass) == false) { | ||
| var moduleName = callerClass.getModule().getName(); | ||
| var notEntitledLogger = LogManager.getLogger(PolicyManager.class + "." + ((moduleName == null) ? "ALL_UNNAMED" : moduleName)); |
There was a problem hiding this comment.
nit: we use ALL-UNNAMED (with a dash) in the policy files
There was a problem hiding this comment.
Why the module name and not the component name? (plugin name, "server" or "apm-agent"?)
Wouldn't we have clash for ALL-UNNAMED from different plugins if we need to adjust log level for one non-modular plugin?
There was a problem hiding this comment.
The component name is also reasonable, but the finer grained we can make this the less changing this logging could affect other log messages.
There was a problem hiding this comment.
Having both as agreed is the best 👍 Thanks!
prdoyle
changed the title
libs/entitlement/src/main/java/org/elasticsearch/entitlement/runtime/policy/PolicyManager.java
Outdated
Show resolved
Hide resolved
We're alerting on this, so let's not rely on every caller of notEntitled to remember it.
* Refactor: findRequestingFrame * INFO instead of WARN for NotEntitledException. Some of these are expected, so an INFO seems more appropriate. The stack trace tends to attract attention even when entitlements are not the cause of a problem, so let's avoid the stack trace, but still include stack frame info from the frame of interest. * Use child loggers for Not Entitled logs * Use warn, and include compoenent name * Fix ALL_UNNAMED * Mute entitlement warnings from repositories * PR feedback * Common out the Not Entitled prefix. We're alerting on this, so let's not rely on every caller of notEntitled to remember it.
* Refactor: findRequestingFrame * INFO instead of WARN for NotEntitledException. Some of these are expected, so an INFO seems more appropriate. The stack trace tends to attract attention even when entitlements are not the cause of a problem, so let's avoid the stack trace, but still include stack frame info from the frame of interest. * Use child loggers for Not Entitled logs * Use warn, and include compoenent name * Fix ALL_UNNAMED * Mute entitlement warnings from repositories * PR feedback * Common out the Not Entitled prefix. We're alerting on this, so let's not rely on every caller of notEntitled to remember it.
* Refactor: findRequestingFrame * INFO instead of WARN for NotEntitledException. Some of these are expected, so an INFO seems more appropriate. The stack trace tends to attract attention even when entitlements are not the cause of a problem, so let's avoid the stack trace, but still include stack frame info from the frame of interest. * Use child loggers for Not Entitled logs * Use warn, and include compoenent name * Fix ALL_UNNAMED * Mute entitlement warnings from repositories * PR feedback * Common out the Not Entitled prefix. We're alerting on this, so let's not rely on every caller of notEntitled to remember it.
* Refactor: findRequestingFrame * INFO instead of WARN for NotEntitledException. Some of these are expected, so an INFO seems more appropriate. The stack trace tends to attract attention even when entitlements are not the cause of a problem, so let's avoid the stack trace, but still include stack frame info from the frame of interest. * Use child loggers for Not Entitled logs * Use warn, and include compoenent name * Fix ALL_UNNAMED * Mute entitlement warnings from repositories * PR feedback * Common out the Not Entitled prefix. We're alerting on this, so let's not rely on every caller of notEntitled to remember it.
* Refactor: findRequestingFrame * INFO instead of WARN for NotEntitledException. Some of these are expected, so an INFO seems more appropriate. The stack trace tends to attract attention even when entitlements are not the cause of a problem, so let's avoid the stack trace, but still include stack frame info from the frame of interest. * Use child loggers for Not Entitled logs * Use warn, and include compoenent name * Fix ALL_UNNAMED * Mute entitlement warnings from repositories * PR feedback * Common out the Not Entitled prefix. We're alerting on this, so let's not rely on every caller of notEntitled to remember it.
* Refactor: findRequestingFrame * INFO instead of WARN for NotEntitledException. Some of these are expected, so an INFO seems more appropriate. The stack trace tends to attract attention even when entitlements are not the cause of a problem, so let's avoid the stack trace, but still include stack frame info from the frame of interest. * Use child loggers for Not Entitled logs * Use warn, and include compoenent name * Fix ALL_UNNAMED * Mute entitlement warnings from repositories * PR feedback * Common out the Not Entitled prefix. We're alerting on this, so let's not rely on every caller of notEntitled to remember it.
* Refactor: findRequestingFrame * INFO instead of WARN for NotEntitledException. Some of these are expected, so an INFO seems more appropriate. The stack trace tends to attract attention even when entitlements are not the cause of a problem, so let's avoid the stack trace, but still include stack frame info from the frame of interest. * Use child loggers for Not Entitled logs * Use warn, and include compoenent name * Fix ALL_UNNAMED * Mute entitlement warnings from repositories * PR feedback * Common out the Not Entitled prefix. We're alerting on this, so let's not rely on every caller of notEntitled to remember it.
4 participants
The stack trace tends to attract attention even when entitlements are not the cause of a problem, so let's avoid the stack trace, but still include stack frame info from the frame of interest. We're in something of a unique position here, where we know the exact frame that matters (except in cases where the problem stems from landing on the wrong frame, I suppose).
Let's use child loggers so we can filter these independently.