Telemetry with two implementations: basic usage logging and opentelemetry.io observability #92

nls-jajuko · 2024-01-22T13:41:24Z

Fixes #90

This pull request adds telemetry to hakunapi.

Add interfaces, configuration parser, factory and a no operations implementation to hakunapi-core in package fi.nls.hakunapi.telemetry.

Example configuration

telemetry.mode=log-json
telemetry.logger=fi.nls.hakunapi.telemetry
telemetry.collections=*
telemetry.fields=username
telemetry.fields.username.header=remote-user

Adds a new module hakunapi-telemetry which adds a Log4j based telemetry that can be used in basic usage logging.

configuration: telemetry.mode=log-json
Example logging

2024-01-22 15:23:16.992|INFO |fi.nls.hakunapi.telemetry|{"example-collection":5,"username":"testuser"}

Adds a new module hakunapi-telemetry-webapp-javax

module contains no code
module is only used for dependencies and telemetry log configurations for basic usage logging
General idea is to keep default webapp clean from extra telemetry dependencies

Adds a new module hakunapi-telemetry-opentelemetry which adds some basic observability to hakunapi requests using https://opentelemetry.io/ .

configuration: telemetry.mode=opentelemetry
Module has not yet been tested with any container, backend or collector
-- https://opentelemetry.io/docs/collector/
-- https://github.com/open-telemetry/opentelemetry-java-instrumentation/tree/main/instrumentation
Implements context propagation to supplement any opentelemetry instrumentations configured in servlet container

Example logging (using io.opentelemetry.exporter.logging.LoggingSpanExporter )

INFO: 'example-collection' : 7d5030e9639f7ccd445179778619a7f7 d0e0ce8d3b9ee4b8 SERVER [tracer: fi.nls.hakunapi.telemetry:] AttributesMap{data={count=5}, capacity=128, totalAddedValues=1}

jampukka · 2024-01-23T07:15:16Z

src/hakunapi-core/src/main/java/fi/nls/hakunapi/core/telemetry/TelemetryConfigParser.java

+        telemetry.setHeaders(headersMap);
+        if (headers != null) {
+            for (String header : headers) {
+                String lookup = parser.get(String.format(TELEMETRY_FIELDS_HEADER, header), header);


Took a while to understand how this works, but it should.

jampukka · 2024-01-23T07:22:01Z

src/hakunapi-core/src/main/java/fi/nls/hakunapi/core/telemetry/TelemetryConfigParser.java

+        if (collectionsWildcard == null) {
+            return ServiceTelemetry.NOP;
+
+        } else if (service != null && "*".equals(collectionsWildcard)) {


if service is null here then everything is probably messed up badly and we should just fail.
Should use TELEMETRY_COLLECTIONS_WILDCARD instead of "*".

jampukka · 2024-01-23T07:28:23Z

src/hakunapi-core/src/main/java/fi/nls/hakunapi/core/telemetry/TelemetrySpan.java

+
+public interface TelemetrySpan extends AutoCloseable {
+
+    default void counts(int count) {};


Not sure if the default implementation should be a NOP. I would've marked these as required to implement and only implemented these as empty functions in the NOP implementation.

I'll remove default method declarations

jampukka · 2024-01-23T07:32:59Z

...main/java/fi/nls/hakunapi/simple/servlet/javax/operation/GetCollectionItemByIdOperation.java

-        request.getFormat().getResponseHeaders(request).forEach((k, v) -> builder.header(k, v));
-        builder.entity(baos.toByteArray());
-        return builder.build();
+            span.counts(1);


If telemetry fails then the whole operation fails. This seems more auditlog-gy (logging the operation is an essential part of the operation) - this is probably by design, just wanted to make sure.

I think telemetry should not prevent operation.
Maybe we'll need a bunch of try catch-all additions to telemetry?

In GetCollectionItemById it's probably simple as we prefetch the feature into memory and always call counts(1), so probably something like:

final Response response = builder.entity(baos.toByteArray()).build(); // Nothing can fail after this line, except the Telemetry, but we can catch it try (TelemetrySpan span = ftt.span()) { span.counts(1); } catch (Exception e) { // log or ignore the exception } return response;

Spans have the ability to record the duration of operations.
We would lose that if we are not wrapping the actual operations with spans.

Maybe there should be a contract that "telemetry shall never fail"?

In GetCollectionItemsOperation is probably a bit trickier due to streaming. I guess we could move WriteReport report to outside of the inner try block, e.g.:

WriteReport report = null; try (FeatureStream features = producer.getFeatures(request, c); FeatureCollectionWriter writer = request.getFormat().getFeatureCollectionWriter()) { ... report = SimpleFeatureWriter.writeFeatureCollection(writer, ft, c.getProperties(), features, request.getOffset(), request.getLimit()); ... } if (report != null) { try (TelemetrySpan span = ftt.span()) { span.counts(report); } catch (Exception e) { // Log or ignore } }

same comment as before - we would lose duration tracking with that change

Oh okay. Yes then it would be best that telemetry implementations can never throw exceptions (checked or unchecked)

kept this as is to have the possibility to trace spans and span durations

jampukka · 2024-01-23T07:36:42Z

...main/java/fi/nls/hakunapi/simple/servlet/javax/operation/GetCollectionItemByIdOperation.java


-        int srid = request.getSRID();
+            int srid = request.getSRID();


Some whitespace changes here make it harder to tell what actually changed in this file. Not necessary to change, should look into moving towards some projection wide linting solution.

Yes, that would be nice.
I had to reindent everything when coding jsonfg due missing indentation settings, sorry about that

jampukka · 2024-01-23T07:39:23Z

...pi-telemetry-opentelemetry/src/main/java/fi/nls/hakunapi/telemetry/TracingConfiguration.java

+public class TracingConfiguration {
+
+    /** The number of milliseconds between metric exports. */
+    private static final long METRIC_EXPORT_INTERVAL_MS = 800L;


Should this be configurable?

Yes
I'll add that to parse()

nls-jajuko · 2024-01-23T09:44:30Z

Added lifecycle methods for ServiceTelemetry and added telemetry to toClose collection in context listener

jampukka · 2024-01-23T11:20:07Z

Looks good to me

nls-jajuko added 7 commits January 22, 2024 10:26

telemetry basics and a simple logging telemetry implementation

4c63bfe

telemetry factory with log-json module

8050ab2

use GetFeatureRequest instead of FeatureType

ffafbb6

hook for additional config options

2a54835

add missing check for collection telemetry

3d7d0d9

alternate telemetry with opentelemetry.io

c4c9e05

span scope and context

275f97f

nls-jajuko added this to the 1.3.0 milestone Jan 22, 2024

nls-jajuko added the enhancement New feature or request label Jan 22, 2024

add opentelemtry module to telemetry enabled webapp

09997d4

nls-jajuko changed the title ~~Telemetry with two implementations logging and opentelemetry.io~~ Telemetry with two implementations: basic usage logging and opentelemetry.io observability Jan 22, 2024

nls-jajuko added 8 commits January 22, 2024 16:31

additional attributes for span

361a7ce

Update LoggingRequestTelemetry.java

46cc0a9

Update LoggingRequestTelemetry.java

6589d71

Update LoggingRequestTelemetry.java

0f90c6b

Update LoggingRequestTelemetry.java

fa49dea

Update LoggingRequestTelemetry.java

e91533f

Update TracingRequestTelemetry.java

7e72c48

Update TracingRequestTelemetry.java

509d9a5

jampukka reviewed Jan 23, 2024

View reviewed changes

fixed based on feedback / add lifecycle methods to ServiceTelemetry

797d0cc

jampukka merged commit 7e098a8 into nlsfi:main Jan 23, 2024
2 checks passed

nls-jajuko deleted the telemetry branch March 25, 2024 09:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Telemetry with two implementations: basic usage logging and opentelemetry.io observability #92

Telemetry with two implementations: basic usage logging and opentelemetry.io observability #92

nls-jajuko commented Jan 22, 2024 •

edited

Loading

jampukka Jan 23, 2024

jampukka Jan 23, 2024

nls-jajuko Jan 23, 2024

jampukka Jan 23, 2024

nls-jajuko Jan 23, 2024

nls-jajuko Jan 23, 2024

jampukka Jan 23, 2024

nls-jajuko Jan 23, 2024

jampukka Jan 23, 2024

nls-jajuko Jan 23, 2024

jampukka Jan 23, 2024 •

edited

Loading

nls-jajuko Jan 23, 2024

jampukka Jan 23, 2024

nls-jajuko Jan 23, 2024

jampukka Jan 23, 2024

nls-jajuko Jan 23, 2024 •

edited

Loading

jampukka Jan 23, 2024

nls-jajuko Jan 23, 2024 •

edited

Loading

nls-jajuko Jan 23, 2024

nls-jajuko commented Jan 23, 2024

jampukka commented Jan 23, 2024


		public interface TelemetrySpan extends AutoCloseable {

		default void counts(int count) {};

Telemetry with two implementations: basic usage logging and opentelemetry.io observability #92

Telemetry with two implementations: basic usage logging and opentelemetry.io observability #92

Conversation

nls-jajuko commented Jan 22, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jampukka Jan 23, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nls-jajuko Jan 23, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nls-jajuko Jan 23, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nls-jajuko commented Jan 23, 2024

jampukka commented Jan 23, 2024

nls-jajuko commented Jan 22, 2024 •

edited

Loading

jampukka Jan 23, 2024 •

edited

Loading

nls-jajuko Jan 23, 2024 •

edited

Loading

nls-jajuko Jan 23, 2024 •

edited

Loading