Skip to content

Commit a5357ef

Browse files
authored
Reduce noise from NotEntitledException logging (elastic#124511) (elastic#124592)
* 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.
1 parent 5d1140e commit a5357ef

File tree

4 files changed

+89
-23
lines changed

4 files changed

+89
-23
lines changed

libs/entitlement/src/main/java/org/elasticsearch/entitlement/runtime/policy/PolicyManager.java

Lines changed: 31 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -247,15 +247,17 @@ private void neverEntitled(Class<?> callerClass, Supplier<String> operationDescr
247247
return;
248248
}
249249

250+
String componentName = getEntitlements(requestingClass).componentName();
250251
notEntitled(
251252
Strings.format(
252-
"Not entitled: component [%s], module [%s], class [%s], operation [%s]",
253-
getEntitlements(requestingClass).componentName(),
253+
"component [%s], module [%s], class [%s], operation [%s]",
254+
componentName,
254255
requestingClass.getModule().getName(),
255256
requestingClass,
256257
operationDescription.get()
257258
),
258-
callerClass
259+
callerClass,
260+
componentName
259261
);
260262
}
261263

@@ -366,13 +368,14 @@ public void checkFileRead(Class<?> callerClass, Path path, boolean followLinks)
366368
if (canRead == false) {
367369
notEntitled(
368370
Strings.format(
369-
"Not entitled: component [%s], module [%s], class [%s], entitlement [file], operation [read], path [%s]",
371+
"component [%s], module [%s], class [%s], entitlement [file], operation [read], path [%s]",
370372
entitlements.componentName(),
371373
requestingClass.getModule().getName(),
372374
requestingClass,
373375
realPath == null ? path : Strings.format("%s -> %s", path, realPath)
374376
),
375-
callerClass
377+
callerClass,
378+
entitlements.componentName()
376379
);
377380
}
378381
}
@@ -395,13 +398,14 @@ public void checkFileWrite(Class<?> callerClass, Path path) {
395398
if (entitlements.fileAccess().canWrite(path) == false) {
396399
notEntitled(
397400
Strings.format(
398-
"Not entitled: component [%s], module [%s], class [%s], entitlement [file], operation [write], path [%s]",
401+
"component [%s], module [%s], class [%s], entitlement [file], operation [write], path [%s]",
399402
entitlements.componentName(),
400403
requestingClass.getModule().getName(),
401404
requestingClass,
402405
path
403406
),
404-
callerClass
407+
callerClass,
408+
entitlements.componentName()
405409
);
406410
}
407411
}
@@ -483,13 +487,14 @@ private void checkFlagEntitlement(
483487
if (classEntitlements.hasEntitlement(entitlementClass) == false) {
484488
notEntitled(
485489
Strings.format(
486-
"Not entitled: component [%s], module [%s], class [%s], entitlement [%s]",
490+
"component [%s], module [%s], class [%s], entitlement [%s]",
487491
classEntitlements.componentName(),
488492
requestingClass.getModule().getName(),
489493
requestingClass,
490494
PolicyParser.getEntitlementTypeName(entitlementClass)
491495
),
492-
callerClass
496+
callerClass,
497+
classEntitlements.componentName()
493498
);
494499
}
495500
logger.debug(
@@ -524,21 +529,29 @@ public void checkWriteProperty(Class<?> callerClass, String property) {
524529
}
525530
notEntitled(
526531
Strings.format(
527-
"Not entitled: component [%s], module [%s], class [%s], entitlement [write_system_properties], property [%s]",
532+
"component [%s], module [%s], class [%s], entitlement [write_system_properties], property [%s]",
528533
entitlements.componentName(),
529534
requestingClass.getModule().getName(),
530535
requestingClass,
531536
property
532537
),
533-
callerClass
538+
callerClass,
539+
entitlements.componentName()
534540
);
535541
}
536542

537-
private void notEntitled(String message, Class<?> callerClass) {
543+
private void notEntitled(String message, Class<?> callerClass, String componentName) {
538544
var exception = new NotEntitledException(message);
539545
// Don't emit a log for muted classes, e.g. classes containing self tests
540546
if (mutedClasses.contains(callerClass) == false) {
541-
logger.warn(message, exception);
547+
var moduleName = callerClass.getModule().getName();
548+
var loggerSuffix = "." + componentName + "." + ((moduleName == null) ? ALL_UNNAMED : moduleName);
549+
var notEntitledLogger = LogManager.getLogger(PolicyManager.class.getName() + loggerSuffix);
550+
String frameInfoSuffix = StackWalker.getInstance(RETAIN_CLASS_REFERENCE)
551+
.walk(this::findRequestingFrame)
552+
.map(frame -> "\n\tat " + frame)
553+
.orElse("");
554+
notEntitledLogger.warn("Not entitled: " + message + frameInfoSuffix);
542555
}
543556
throw exception;
544557
}
@@ -658,19 +671,18 @@ Class<?> requestingClass(Class<?> callerClass) {
658671
return callerClass;
659672
}
660673
Optional<Class<?>> result = StackWalker.getInstance(RETAIN_CLASS_REFERENCE)
661-
.walk(frames -> findRequestingClass(frames.map(StackFrame::getDeclaringClass)));
674+
.walk(frames -> findRequestingFrame(frames).map(StackFrame::getDeclaringClass));
662675
return result.orElse(null);
663676
}
664677

665678
/**
666-
* Given a stream of classes corresponding to the frames from a {@link StackWalker},
667-
* returns the module whose entitlements should be checked.
679+
* Given a stream of {@link StackFrame}s, identify the one whose entitlements should be checked.
668680
*
669681
* @throws NullPointerException if the requesting module is {@code null}
670682
*/
671-
Optional<Class<?>> findRequestingClass(Stream<Class<?>> classes) {
672-
return classes.filter(c -> c.getModule() != entitlementsModule) // Ignore the entitlements library
673-
.skip(1) // Skip the sensitive caller method
683+
Optional<StackFrame> findRequestingFrame(Stream<StackFrame> frames) {
684+
return frames.filter(f -> f.getDeclaringClass().getModule() != entitlementsModule) // ignore entitlements library
685+
.skip(1) // Skip the sensitive caller method
674686
.findFirst();
675687
}
676688

libs/entitlement/src/test/java/org/elasticsearch/entitlement/runtime/policy/PolicyManagerTests.java

Lines changed: 51 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import org.junit.BeforeClass;
2424

2525
import java.io.IOException;
26+
import java.lang.StackWalker.StackFrame;
2627
import java.lang.module.Configuration;
2728
import java.lang.module.ModuleFinder;
2829
import java.net.URL;
@@ -320,18 +321,21 @@ public void testRequestingModuleWithStackWalk() throws IOException, ClassNotFoun
320321
assertEquals(
321322
"Skip entitlement library and the instrumented method",
322323
requestingClass,
323-
policyManager.findRequestingClass(Stream.of(entitlementsClass, instrumentedClass, requestingClass, ignorableClass)).orElse(null)
324+
policyManager.findRequestingFrame(
325+
Stream.of(entitlementsClass, instrumentedClass, requestingClass, ignorableClass).map(MockFrame::new)
326+
).map(StackFrame::getDeclaringClass).orElse(null)
324327
);
325328
assertEquals(
326329
"Skip multiple library frames",
327330
requestingClass,
328-
policyManager.findRequestingClass(Stream.of(entitlementsClass, entitlementsClass, instrumentedClass, requestingClass))
329-
.orElse(null)
331+
policyManager.findRequestingFrame(
332+
Stream.of(entitlementsClass, entitlementsClass, instrumentedClass, requestingClass).map(MockFrame::new)
333+
).map(StackFrame::getDeclaringClass).orElse(null)
330334
);
331335
assertThrows(
332336
"Non-modular caller frames are not supported",
333337
NullPointerException.class,
334-
() -> policyManager.findRequestingClass(Stream.of(entitlementsClass, null))
338+
() -> policyManager.findRequestingFrame(Stream.of(entitlementsClass, null).map(MockFrame::new))
335339
);
336340
}
337341

@@ -657,4 +661,47 @@ private static ModuleLayer createLayerForJar(Path jar, String moduleName) {
657661
);
658662
return moduleController.layer();
659663
}
664+
665+
record MockFrame(Class<?> declaringClass) implements StackFrame {
666+
@Override
667+
public String getClassName() {
668+
return getDeclaringClass().getName();
669+
}
670+
671+
@Override
672+
public String getMethodName() {
673+
throw new UnsupportedOperationException();
674+
}
675+
676+
@Override
677+
public Class<?> getDeclaringClass() {
678+
return declaringClass;
679+
}
680+
681+
@Override
682+
public int getByteCodeIndex() {
683+
throw new UnsupportedOperationException();
684+
}
685+
686+
@Override
687+
public String getFileName() {
688+
throw new UnsupportedOperationException();
689+
}
690+
691+
@Override
692+
public int getLineNumber() {
693+
throw new UnsupportedOperationException();
694+
}
695+
696+
@Override
697+
public boolean isNativeMethod() {
698+
throw new UnsupportedOperationException();
699+
}
700+
701+
@Override
702+
public StackTraceElement toStackTraceElement() {
703+
throw new UnsupportedOperationException();
704+
}
705+
}
706+
660707
}
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
logger.org_elasticsearch_entitlement_runtime_policy_PolicyManager.name = org.elasticsearch.entitlement.runtime.policy.PolicyManager.repository-gcs.ALL-UNNAMED
2+
logger.org_elasticsearch_entitlement_runtime_policy_PolicyManager.level = error
3+

modules/repository-s3/src/main/config/log4j2.properties

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,3 +12,7 @@ logger.com_amazonaws_auth_profile_internal_BasicProfileConfigFileLoader.level =
1212

1313
logger.com_amazonaws_services_s3_internal_UseArnRegionResolver.name = com.amazonaws.services.s3.internal.UseArnRegionResolver
1414
logger.com_amazonaws_services_s3_internal_UseArnRegionResolver.level = error
15+
16+
logger.org_elasticsearch_entitlement_runtime_policy_PolicyManager.name = org.elasticsearch.entitlement.runtime.policy.PolicyManager.repository-s3.software.amazon.awssdk.profiles
17+
logger.org_elasticsearch_entitlement_runtime_policy_PolicyManager.level = error
18+

0 commit comments

Comments
 (0)