Skip to content

Commit

Permalink
Shorten Observation Event Names
Browse files Browse the repository at this point in the history 
Closes gh-12811
  • Loading branch information
@bvn13 @jzheaux
bvn13 authored and jzheaux committed Apr 12, 2023
1 parent b3c8344 commit 59ba7f5
Show file tree
Hide file tree
Showing 2 changed files with 174 additions and 2 deletions.
155 changes: 153 additions & 2 deletions web/src/main/java/org/springframework/security/web/ObservationFilterChainDecorator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,9 @@

import java.io.IOException;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.concurrent.atomic.AtomicReference;

Expand All @@ -36,6 +38,36 @@
import org.apache.commons.logging.LogFactory;

import org.springframework.core.log.LogMessage;
import org.springframework.security.web.access.ExceptionTranslationFilter;
import org.springframework.security.web.access.channel.ChannelProcessingFilter;
import org.springframework.security.web.access.intercept.AuthorizationFilter;
import org.springframework.security.web.access.intercept.FilterSecurityInterceptor;
import org.springframework.security.web.authentication.AnonymousAuthenticationFilter;
import org.springframework.security.web.authentication.UsernamePasswordAuthenticationFilter;
import org.springframework.security.web.authentication.logout.LogoutFilter;
import org.springframework.security.web.authentication.preauth.AbstractPreAuthenticatedProcessingFilter;
import org.springframework.security.web.authentication.preauth.x509.X509AuthenticationFilter;
import org.springframework.security.web.authentication.rememberme.RememberMeAuthenticationFilter;
import org.springframework.security.web.authentication.switchuser.SwitchUserFilter;
import org.springframework.security.web.authentication.ui.DefaultLoginPageGeneratingFilter;
import org.springframework.security.web.authentication.ui.DefaultLogoutPageGeneratingFilter;
import org.springframework.security.web.authentication.www.BasicAuthenticationFilter;
import org.springframework.security.web.authentication.www.DigestAuthenticationFilter;
import org.springframework.security.web.context.SecurityContextHolderFilter;
import org.springframework.security.web.context.SecurityContextPersistenceFilter;
import org.springframework.security.web.context.request.async.WebAsyncManagerIntegrationFilter;
import org.springframework.security.web.csrf.CsrfFilter;
import org.springframework.security.web.header.HeaderWriterFilter;
import org.springframework.security.web.jaasapi.JaasApiIntegrationFilter;
import org.springframework.security.web.savedrequest.RequestCacheAwareFilter;
import org.springframework.security.web.servletapi.SecurityContextHolderAwareRequestFilter;
import org.springframework.security.web.session.ConcurrentSessionFilter;
import org.springframework.security.web.session.DisableEncodeUrlFilter;
import org.springframework.security.web.session.ForceEagerSessionCreationFilter;
import org.springframework.security.web.session.SessionManagementFilter;
import org.springframework.util.Assert;
import org.springframework.util.StringUtils;
import org.springframework.web.filter.CorsFilter;

/**
* A {@link org.springframework.security.web.FilterChainProxy.FilterChainDecorator} that
Expand All @@ -48,6 +80,12 @@ public final class ObservationFilterChainDecorator implements FilterChainProxy.F

private static final Log logger = LogFactory.getLog(FilterChainProxy.class);

private static final int OPENTELEMETRY_MAX_NAME_LENGTH = 63;

private static final int MAX_OBSERVATION_NAME_LENGTH = OPENTELEMETRY_MAX_NAME_LENGTH - ".before".length();

private static final Map<String, String> OBSERVATION_NAMES = new HashMap<>();

private static final String ATTRIBUTE = ObservationFilterChainDecorator.class + ".observation";

static final String UNSECURED_OBSERVATION_NAME = "spring.security.http.unsecured.requests";
Expand Down Expand Up @@ -98,10 +136,83 @@ private List<ObservationFilter> wrap(List<Filter> filters) {
return observableFilters;
}

static {
registerName(DisableEncodeUrlFilter.class, "session.encode-url.disable");
registerName(ForceEagerSessionCreationFilter.class, "session.create");
registerName(ChannelProcessingFilter.class, "web.request.delivery.ensure");
registerName(WebAsyncManagerIntegrationFilter.class, "web-async-manager.join.security-context");
registerName(SecurityContextHolderFilter.class, "security-context.hold");
registerName(SecurityContextPersistenceFilter.class, "security-context.persist");
registerName(HeaderWriterFilter.class, "web.response.header.set");
registerName(CorsFilter.class, "cors.process");
registerName(CsrfFilter.class, "csrf.protect");
registerName(LogoutFilter.class, "principal.logout");
registerName("org.springframework.security.oauth2.client.web.OAuth2AuthorizationRequestRedirectFilter",
"web.request.oauth2.redirect");
registerName(
"org.springframework.security.saml2.provider.service.web." + "Saml2WebSsoAuthenticationRequestFilter",
"web.request.saml2.redirect");
registerName(X509AuthenticationFilter.class, "web.request.x509.auth");
registerName(AbstractPreAuthenticatedProcessingFilter.class, "web.request.pre-auth.base.process");
registerName("org.springframework.security.cas.web.CasAuthenticationFilter", "web.request.sas.auth");
registerName("org.springframework.security.oauth2.client.web.OAuth2LoginAuthenticationFilter",
"web.response.oauth2.process");
registerName("org.springframework.security.saml2.provider.service.web.authentication"
+ ".Saml2WebSsoAuthenticationFilter", "web.request.saml2.auth");
registerName(UsernamePasswordAuthenticationFilter.class, "web.request.username-password.auth");
registerName(DefaultLoginPageGeneratingFilter.class, "web.login-page.default.generate");
registerName(DefaultLogoutPageGeneratingFilter.class, "web.logout-page.default.generate");
registerName(ConcurrentSessionFilter.class, "session.refresh");
registerName(DigestAuthenticationFilter.class, "web.request.digest.auth");
registerName("org.springframework.security.oauth2.server.resource.web.authentication."
+ "BearerTokenAuthenticationFilter", "web.request.bearer.auth");
registerName(BasicAuthenticationFilter.class, "web.request.basic.auth");
registerName(RequestCacheAwareFilter.class, "web.request.cache.extract");
registerName(SecurityContextHolderAwareRequestFilter.class, "web.request.security.wrap");
registerName(JaasApiIntegrationFilter.class, "web.request.jass.auth");
registerName(RememberMeAuthenticationFilter.class, "web.request.remember-me.auth");
registerName(AnonymousAuthenticationFilter.class, "web.request.anonymous.auth");
registerName("org.springframework.security.oauth2.client.web.OAuth2AuthorizationCodeGrantFilter",
"web.response.oauth2.code-grant.process");
registerName(SessionManagementFilter.class, "session.manage");
registerName(ExceptionTranslationFilter.class, "exception.translate");
registerName(FilterSecurityInterceptor.class, "web.response.security.intercept");
registerName(AuthorizationFilter.class, "web.access.auth.restrict");
registerName(SwitchUserFilter.class, "session.switch");
}

public static void registerName(Class clazz, String name) {
String keyName = clazz.getName();
checkAlreadyRegistered(keyName);
OBSERVATION_NAMES.put(keyName, limitLength(name));
}

public static void registerName(String className, String name) {
checkAlreadyRegistered(className);
OBSERVATION_NAMES.put(className, name);
}

static AroundFilterObservation observation(HttpServletRequest request) {
return (AroundFilterObservation) request.getAttribute(ATTRIBUTE);
}

private static String getObservationName(String className) {
if (OBSERVATION_NAMES.containsKey(className)) {
return OBSERVATION_NAMES.get(className);
}
throw new IllegalArgumentException("Class not registered for observation: " + className);
}

private static String limitLength(String s) {
Assert.isTrue(s.length() <= MAX_OBSERVATION_NAME_LENGTH,
"The name must be less than MAX_OBSERVATION_NAME_LENGTH=" + MAX_OBSERVATION_NAME_LENGTH);
return s;
}

private static void checkAlreadyRegistered(String keyName) {
Assert.isTrue(!OBSERVATION_NAMES.containsKey(keyName), "Observation name is registered already: " + keyName);
}

private static final class VirtualFilterChain implements FilterChain {

private final FilterChain originalChain;
Expand Down Expand Up @@ -145,6 +256,8 @@ static final class ObservationFilter implements Filter {

private final String name;

private final String observationName;

private final int position;

private final int size;
Expand All @@ -155,12 +268,30 @@ static final class ObservationFilter implements Filter {
this.name = filter.getClass().getSimpleName();
this.position = position;
this.size = size;
String tempObservationName;
try {
tempObservationName = ObservationFilterChainDecorator.getObservationName(filter.getClass().getName());
}
catch (IllegalArgumentException ex) {
tempObservationName = compressName(this.name);
logger.warn(
"Class " + filter.getClass().getName()
+ " is not registered for observation and will have name " + tempObservationName
+ ". Please consider of registering this class with "
+ ObservationFilterChainDecorator.class.getSimpleName() + ".registerName(class, name).",
ex);
}
this.observationName = tempObservationName;
}

String getName() {
return this.name;
}

String getObservationName() {
return this.observationName;
}

@Override
public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain)
throws IOException, ServletException {
Expand All @@ -181,15 +312,17 @@ private void wrapFilter(ServletRequest request, ServletResponse response, Filter
parentBefore.setFilterName(this.name);
parentBefore.setChainPosition(this.position);
}
parent.before().event(Observation.Event.of(this.name + ".before", "before " + this.name));
parent.before().event(Observation.Event.of(this.observationName + ".before",
"before " + this.name));
this.filter.doFilter(request, response, chain);
parent.start();
if (parent.after().getContext() instanceof FilterChainObservationContext parentAfter) {
parentAfter.setChainSize(this.size);
parentAfter.setFilterName(this.name);
parentAfter.setChainPosition(this.size - this.position + 1);
}
parent.after().event(Observation.Event.of(this.name + ".after", "after " + this.name));
parent.after().event(Observation.Event.of(this.observationName + ".after",
"after " + this.name));
}

private AroundFilterObservation parent(HttpServletRequest request) {
Expand All @@ -202,6 +335,24 @@ private AroundFilterObservation parent(HttpServletRequest request) {
return parent;
}

private String compressName(String className) {
if (className.length() >= MAX_OBSERVATION_NAME_LENGTH) {
return maximalCompressClassName(className, MAX_OBSERVATION_NAME_LENGTH);
}
return className;
}

private String maximalCompressClassName(String className, int maxLength) {
String[] names = className.split("(?=\\p{Lu})");
for (int j = 0; j < names.length; j++) {
final int maxPortionLength = maxLength / names.length;
if (names[j].length() > maxPortionLength) {
names[j] = names[j].substring(0, maxPortionLength);
}
}
return StringUtils.arrayToDelimitedString(names, "");
}

}

interface AroundFilterObservation extends FilterObservation {
Expand Down
21 changes: 21 additions & 0 deletions web/src/test/java/org/springframework/security/web/ObservationFilterChainDecoratorTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@

package org.springframework.security.web;

import java.lang.reflect.Field;
import java.lang.reflect.Method;
import java.util.List;

import io.micrometer.observation.Observation;
Expand Down Expand Up @@ -78,6 +80,7 @@ void decorateFiltersWhenDefaultsThenObserves() throws Exception {
FilterChain chain = mock(FilterChain.class);
Filter filter = mock(Filter.class);
FilterChain decorated = decorator.decorate(chain, List.of(filter));
assertCompressedName(decorated);
decorated.doFilter(new MockHttpServletRequest("GET", "/"), new MockHttpServletResponse());
verify(handler, times(2)).onStart(any());
ArgumentCaptor<Observation.Event> event = ArgumentCaptor.forClass(Observation.Event.class);
Expand All @@ -87,4 +90,22 @@ void decorateFiltersWhenDefaultsThenObserves() throws Exception {
assertThat(events.get(1).getName()).isEqualTo(filter.getClass().getSimpleName() + ".after");
}

void assertCompressedName(FilterChain filterChain) throws Exception {
assertThat(filterChain.getClass().getSimpleName()).isEqualTo("VirtualFilterChain");
Field field = filterChain.getClass().getDeclaredField("additionalFilters");
field.setAccessible(true);
List<ObservationFilterChainDecorator.ObservationFilter> additionalFilters =
(List<ObservationFilterChainDecorator.ObservationFilter>) field.get(filterChain);
assertThat(additionalFilters.size()).isEqualTo(1);
final ObservationFilterChainDecorator.ObservationFilter observationFilter = additionalFilters.get(0);
assertThat(observationFilter.getObservationName()).isEqualTo(observationFilter.getName());
Method method = observationFilter.getClass().getDeclaredMethod("compressName", String.class);
method.setAccessible(true);
String compressed = (String) method.invoke(observationFilter, "ObservationFilterChainDecoratorTests");
assertThat(compressed).isEqualTo("ObservationFilterChainDecoratorTests");
String fakeCompressed = (String) method.invoke(observationFilter,
"ObservationFilterChainDecoratorTestsObservationFilterChainDecoratorTests");
assertThat(fakeCompressed).isEqualTo("ObserFilteChainDecorTestsObserFilteChainDecorTests");
}

}

0 comments on commit 59ba7f5

Please sign in to comment.