Observability support for gateway #2715

marcingrzejszczak · 2022-08-26T15:47:21Z

No description provided.

spencergibb · 2022-08-26T15:49:02Z

pom.xml

@@ -57,10 +57,26 @@
 		<spring-cloud-circuitbreaker.version>3.0.0-SNAPSHOT</spring-cloud-circuitbreaker.version>
 		<spring-cloud-commons.version>4.0.0-SNAPSHOT</spring-cloud-commons.version>
 		<testcontainers.version>1.17.3</testcontainers.version>
+		<micrometer.version>1.10.0-SNAPSHOT</micrometer.version>


What versions does boot manage?

1.10.0-M3 right now but I have an open PR for M4.
They also want to move to SNAPSHOT ~in September I guess.

tony-clarke-amdocs · 2022-08-26T15:58:12Z

How does relate to spring cloud sleuth?

spencergibb · 2022-08-26T16:19:53Z

Only indirectly as the new micrometer functionality replaces sleuth

jonatan-ivanov · 2022-08-26T17:21:33Z

...gframework/cloud/gateway/filter/headers/observation/DefaultGatewayObservationConvention.java

+		if (context.getCarrier() == null) {
+			return keyValues;
+		}
+		// TODO: URI is high cardinality


Do we have the pattern/template available instead of the raw URI?

jonatan-ivanov · 2022-08-26T17:33:33Z

...gframework/cloud/gateway/filter/headers/observation/DefaultGatewayObservationConvention.java

+		// TODO: URI is high cardinality
+		keyValues = keyValues.and(METHOD.withValue(context.getRequest().getMethod().name()), URI.withValue(context.getRequest().getURI().toString()));
+		if (context.getResponse() != null && context.getResponse().getStatusCode() != null) {
+			keyValues = keyValues.and(STATUS.withValue(String.valueOf(context.getResponse().getStatusCode().value())));


This can lead to an interesting issue, please bear with me.
If the status code does not present (e.g.: timeout?) this will not add a status key-value which means that in those scenarios this tag will be missing from the Timer.

This can cause issues in certain backends (i.e.: Prometheus) since it will lead to time series where the name is the same but the tags are different, e.g.:

http_client_requests{method="GET", status="200"} ... http_client_requests{method="GET"} ...

So I guess my question is: will the status code always be there if we already initiated the call (so we are not handling the LongTaskTimer case)?
If so, with some default value (0, -1), we are good. But if not, I would do this:

else { keyValues = keyValues.and(STATUS.withValue("n/a")); }

...gframework/cloud/gateway/filter/headers/observation/DefaultGatewayObservationConvention.java

...ringframework/cloud/gateway/filter/headers/observation/ObservedRequestHttpHeadersFilter.java

jonatan-ivanov · 2022-08-26T17:41:59Z

...ringframework/cloud/gateway/filter/headers/observation/ObservedRequestHttpHeadersFilter.java

+		if (parentObservation != null) {
+			childObservation = childObservation.parentObservation(parentObservation);
+		}
+		childObservation = childObservation.start();


Can we add a comment about where to find stop so that whoever reads this will be able to easily navigate to the other half of this?

...ingframework/cloud/gateway/filter/headers/observation/ObservedResponseHttpHeadersFilter.java

jonatan-ivanov · 2022-08-26T17:43:25Z

...ingframework/cloud/gateway/filter/headers/observation/ObservedResponseHttpHeadersFilter.java

+		if (log.isDebugEnabled()) {
+			log.debug("The response was handled for observation " + childObservation);
+		}
+		childObservation.stop();


Can we add a comment here that tells the reader where to find start?

jonatan-ivanov · 2022-08-26T17:44:19Z

...ingframework/cloud/gateway/filter/headers/observation/ObservedResponseHttpHeadersFilter.java

+ * @author Marcin Grzejszczak
+ * @since 3.0.0
+ */
+public class ObservedResponseHttpHeadersFilter implements HttpHeadersFilter {


What happens if there was a network error and we don't have a response (e.g.: timeout, connection reset, etc)?

…template URL

spencergibb · 2022-10-03T18:36:56Z

...gframework/cloud/gateway/filter/headers/observation/DefaultGatewayObservationConvention.java

+		}
+		ServerWebExchange exchange = context.getExchange();
+		Map<String, String> uriTemplateVariables = ServerWebExchangeUtils.getUriTemplateVariables(exchange);
+		String url = exchange.getAttribute(ServerWebExchangeUtils.GATEWAY_REQUEST_URL_ATTR);


There are two options, the one used here or:

Route route = exchange.getAttribute(GATEWAY_ROUTE_ATTR); URI routeUri = route.getUri();

This is GATEWAY_REQUEST_URL_ATTR is merged with the request URI, the one from the Route object is as configured.

spencergibb · 2022-10-03T18:40:25Z

...ringframework/cloud/gateway/filter/headers/observation/ObservedRequestHttpHeadersFilter.java

+		if (log.isDebugEnabled()) {
+			log.debug("Will instrument the HTTP request headers " + newHeaders);
+		}
+		Observation parentObservation = exchange.getAttribute("micrometer.observation");


Where does "micrometer.observation" come from?

It comes from https://github.com/micrometer-metrics/micrometer/blob/main/micrometer-observation/src/main/java/io/micrometer/observation/contextpropagation/ObservationThreadLocalAccessor.java#L37 - we can make a direct reference to that but then the context propagation dependency would be a mandatory one. I don't really see a problem with that

spencergibb

Builds are failing with

java.lang.ClassCastException: class java.net.URI cannot be cast to class java.lang.String (java.net.URI and java.lang.String are in module java.base of loader 'bootstrap')
	at org.springframework.cloud.gateway.filter.headers.observation.DefaultGatewayObservationConvention.getLowCardinalityKeyValues(DefaultGatewayObservationConvention.java:54) ~[classes/:na]

proteintogo · 2023-01-26T10:38:36Z

I added the following dependency micrometer-tracing-bridge-brave along with spring-cloud-starter-gateway:

Configured the logging.pattern.level=%clr(%5p) %clr([TraceId=%X{traceId:-},SpanId=%X{spanId:-}]){green}

But when I log any message, no traceId/SpanId is completed. Should I do any additional step/configuration ?

marcingrzejszczak · 2023-01-26T13:03:21Z

Check this doc https://micrometer.io/docs/observation#instrumentation_of_reactive_libraries and if you don't want to do anything manually you have to wait until we patch reactor and boot to support this out of the box

taxone · 2023-05-04T08:45:50Z

@marcingrzejszczak I added;

Hooks.enableAutomaticContextPropagation();

but traceIds do not appear in the logs. I created a sample project to reproduce.
Any help would be much appreciated, thanks in advance.

marcingrzejszczak added the enhancement label Aug 26, 2022

marcingrzejszczak added this to the 4.0.0-M3 milestone Aug 26, 2022

spencergibb reviewed Aug 26, 2022

View reviewed changes

jonatan-ivanov reviewed Aug 26, 2022

View reviewed changes

jonatan-ivanov mentioned this pull request Aug 27, 2022

Spring Security Observability for Authentication and Authorization spring-projects/spring-security#11595

Closed

spencergibb modified the milestones: 4.0.0-M3, 4.0.0-M5 Sep 21, 2022

WIP - tests are failing because I don't know which attribute holds a …

14e7d72

…template URL

marcingrzejszczak force-pushed the observability branch from 88f2497 to 26282f9 Compare October 3, 2022 13:25

used GATEWAY_REQUEST_URL_ATTR

709ee78

marcingrzejszczak force-pushed the observability branch from 26282f9 to 709ee78 Compare October 3, 2022 13:27

spencergibb reviewed Oct 3, 2022

View reviewed changes

spencergibb requested changes Oct 3, 2022

View reviewed changes

marcingrzejszczak added 2 commits October 3, 2022 22:41

Destination URI is low cardinality

8e51516

Polish

7592b2a

marcingrzejszczak marked this pull request as ready for review October 4, 2022 10:55

marcingrzejszczak added 2 commits October 4, 2022 13:51

PolisH

f1a6297

Changed key names and added documentation generation

73a4816

spencergibb changed the title ~~First pass on Observability support~~ Observability support for gateway Oct 4, 2022

spencergibb merged commit f2192b0 into main Oct 4, 2022

jonatan-ivanov deleted the observability branch January 26, 2023 18:09

Observability support for gateway #2715

Observability support for gateway #2715

Uh oh!

Conversation

marcingrzejszczak commented Aug 26, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tony-clarke-amdocs commented Aug 26, 2022

Uh oh!

spencergibb commented Aug 26, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

marcingrzejszczak Oct 4, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

spencergibb left a comment

Choose a reason for hiding this comment

Uh oh!

proteintogo commented Jan 26, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

marcingrzejszczak commented Jan 26, 2023

Uh oh!

taxone commented May 4, 2023

Uh oh!

Uh oh!

marcingrzejszczak Oct 4, 2022 •

edited

Loading

proteintogo commented Jan 26, 2023 •

edited

Loading