feat: adding traces to UF components #1763

franciscolopezsancho · 2023-08-10T10:06:21Z

References https://github.com/lightbend/kalix/issues/9380

franciscolopezsancho · 2023-09-12T10:00:52Z

I still need to fix a couple of things but it's reviewable already.

octonato

I did a first pass. Mainly on the codegen part. Left some comments.

...protobuf/src/main/scala/kalix/javasdk/impl/eventsourcedentity/EventSourcedEntityRouter.scala

codegen/java-gen/src/main/scala/kalix/codegen/java/EventSourcedEntitySourceGenerator.scala

sdk/java-sdk-spring/src/main/scala/kalix/javasdk/impl/Telemetry.scala

...rc/main/scala/kalix/javasdk/impl/eventsourcedentity/ReflectiveEventSourcedEntityRouter.scala

franciscolopezsancho · 2023-09-17T12:51:09Z

The new integration test won't pass until https://github.com/lightbend/kalix-proxy/pull/2096 is merged

sdk/java-sdk-protobuf/src/main/resources/reference.conf

sdk/java-sdk-protobuf/src/main/scala/kalix/javasdk/impl/action/ActionsImpl.scala

sdk/java-sdk-protobuf/src/main/scala/kalix/javasdk/impl/telemetry/Telemetry.scala

sdk/java-sdk-spring/docker-compose-integration.yml

sdk/java-sdk-spring/src/it/java/com/example/wiring/tracing/TracingIntegratonTest.java

...rc/main/scala/kalix/javasdk/impl/eventsourcedentity/ReflectiveEventSourcedEntityRouter.scala

sdk/java-sdk-spring/src/main/scala/kalix/javasdk/impl/action/ReflectiveActionRouter.scala

johanandren

Do you expect to try tackle propagating the context to whatever the UF does separately, and this only adds spans for the SDK side of the components? Are we sure we are interested in the SDK spans or is the propagation possibly the valuable thing?

I think I mentioned this in the other PR but separate SDK spans for calls we already have spans for in the proxy is probably more interesting to us than it is to the users.

Compare the value for a user of:

    [-------- proxy --------]
      [------- sdk --------]

Vs

   [-----   service/component A -----------------------------------------]
          [ ---- service/component B]   [----- service/component C –––]

Where they can see what component talked to what other component and which of them took time etc.

sdk/java-sdk-protobuf/src/main/resources/reference.conf

sdk/java-sdk-protobuf/src/main/scala/kalix/javasdk/impl/action/ActionsImpl.scala

...protobuf/src/main/scala/kalix/javasdk/impl/eventsourcedentity/EventSourcedEntitiesImpl.scala

...protobuf/src/main/scala/kalix/javasdk/impl/eventsourcedentity/EventSourcedEntityRouter.scala

sdk/java-sdk-protobuf/src/main/scala/kalix/javasdk/impl/telemetry/Telemetry.scala

aludwiko

minor comments

sdk/java-sdk-protobuf/src/main/scala/kalix/javasdk/impl/DiscoveryImpl.scala

...spring/src/it/java/com/example/wiring/eventsourcedentities/tracingcounter/TCounterEvent.java

johanandren · 2023-10-12T15:03:37Z

sdk/java-sdk-spring/docker-compose-integration.yml

@@ -1,7 +1,7 @@
 version: "3"
 services:
  kalix-proxy:
-    image: gcr.io/kalix-public/kalix-proxy:1.1.19
+    image: gcr.io/kalix-workbench/kalix-proxy:1.1.20


If these changes should stay, it is probably be the official image?

Also makes me wonder a bit if this shouldn't be a separate test module for tracing so that running other it tests for the SDK doesn't necessarily require running the additional containers etc? Maybe talk to @aludwiko or @octonato a bit about it just to be sure?

darn, I thought I changed that back.

Sure. Looking for your feedback, guys.

... I did change it. But didn't push. Not very useful 'it works in my machine'

sdk/java-sdk-spring/tempo-query.yaml

with one ESE and one Action subscribed to that entity.

moving telemetry to java-sdk-protobuf where there is more data to add and the span length is (hopefully) now correct

- Closing the span after the future is complete - Creating only one thread to close/flush all the OpenTelemetrySdks - Some cleaning - Creating in a map of OpenTelemetrySdks to reuse by service

franciscolopezsancho · 2023-10-13T11:43:24Z

sdk/java-sdk-spring/src/it/java/com/example/wiring/pubsub/DockerIntegrationTest.java

@@ -52,18 +50,27 @@ public abstract class DockerIntegrationTest {

  private KalixSpringApplication kalixSpringApplication;

-  public DockerIntegrationTest(ApplicationContext applicationContext) {
+


This changes are related to the TracingIntegrationTest. I needed to pass some extra configuration.

johanandren

One more thing then I think this is fine to merge

...protobuf/src/main/scala/kalix/javasdk/impl/eventsourcedentity/EventSourcedEntitiesImpl.scala

github-actions bot added java-sdk java-sdk-protobuf kalix-runtime Runtime and SDKs sub-team labels Aug 10, 2023

github-actions bot added the scala-sdk-protobuf label Sep 10, 2023

franciscolopezsancho force-pushed the tracing_proxy_to_uf_components branch from 8ffee9e to f23a95b Compare September 12, 2023 08:45

franciscolopezsancho marked this pull request as ready for review September 12, 2023 09:37

franciscolopezsancho requested a review from johanandren September 12, 2023 09:37

octonato reviewed Sep 13, 2023

View reviewed changes

...protobuf/src/main/scala/kalix/javasdk/impl/eventsourcedentity/EventSourcedEntityRouter.scala Outdated Show resolved Hide resolved

codegen/java-gen/src/main/scala/kalix/codegen/java/EventSourcedEntitySourceGenerator.scala Outdated Show resolved Hide resolved

johanandren reviewed Sep 13, 2023

View reviewed changes

johanandren reviewed Sep 18, 2023

View reviewed changes

johanandren reviewed Oct 6, 2023

View reviewed changes

johanandren reviewed Oct 9, 2023

View reviewed changes

sdk/java-sdk-protobuf/src/main/scala/kalix/javasdk/impl/telemetry/Telemetry.scala Outdated Show resolved Hide resolved

sdk/java-sdk-protobuf/src/main/scala/kalix/javasdk/impl/telemetry/Telemetry.scala Outdated Show resolved Hide resolved

johanandren reviewed Oct 11, 2023

View reviewed changes

sdk/java-sdk-protobuf/src/main/scala/kalix/javasdk/impl/telemetry/Telemetry.scala Outdated Show resolved Hide resolved

aludwiko reviewed Oct 12, 2023

View reviewed changes

sdk/java-sdk-protobuf/src/main/scala/kalix/javasdk/impl/DiscoveryImpl.scala Outdated Show resolved Hide resolved

...spring/src/it/java/com/example/wiring/eventsourcedentities/tracingcounter/TCounterEvent.java Outdated Show resolved Hide resolved

franciscolopezsancho force-pushed the tracing_proxy_to_uf_components branch from c7a4a94 to 1ca8915 Compare October 12, 2023 09:35

johanandren reviewed Oct 12, 2023

View reviewed changes

franciscolopezsancho added 12 commits October 13, 2023 14:05

feat: adding traces to UF components

8808174

Passing integration test

fdf92c6

with one ESE and one Action subscribed to that entity.

fixing codegen

4506323

adding headers

9757335

removing initial tests for json parsing

c503242

fmt

97f2327

Removing the previous metadata addition

bbdc9a5

removing unwanted files

05246ca

removing unnecessary metadata

6d40c87

left overs

a63fdf7

addressing comments

9ce8576

moving telemetry to java-sdk-protobuf where there is more data to add and the span length is (hopefully) now correct

Removing imports

b113bf8

franciscolopezsancho added 9 commits October 13, 2023 14:05

addressing comments

3b6ffca

- Closing the span after the future is complete - Creating only one thread to close/flush all the OpenTelemetrySdks - Some cleaning - Creating in a map of OpenTelemetrySdks to reuse by service

some cleaning

f7c4a72

addressing comments and some cleaning

452709a

addressing comments

054d1e9

typo

772be4c

addressing comments and some refactoring

a11ffb7

Update Dependencies.scala

48c6710

addressing comments

7e1453d

Deleting reference to workbench

9ffa1de

franciscolopezsancho force-pushed the tracing_proxy_to_uf_components branch from 508ad85 to 9ffa1de Compare October 13, 2023 08:35

franciscolopezsancho requested a review from johanandren October 13, 2023 11:35

franciscolopezsancho commented Oct 13, 2023

View reviewed changes

redundant

e9cb98f

johanandren reviewed Oct 17, 2023

View reviewed changes

...protobuf/src/main/scala/kalix/javasdk/impl/eventsourcedentity/EventSourcedEntitiesImpl.scala Outdated Show resolved Hide resolved

franciscolopezsancho added 2 commits October 17, 2023 15:13

starting span before decoding

7d05834

fmt

905af4c

johanandren approved these changes Oct 17, 2023

View reviewed changes

franciscolopezsancho merged commit 827d223 into lightbend:main Oct 17, 2023
65 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: adding traces to UF components #1763

feat: adding traces to UF components #1763

franciscolopezsancho commented Aug 10, 2023

franciscolopezsancho commented Sep 12, 2023

octonato left a comment

franciscolopezsancho commented Sep 17, 2023

johanandren left a comment

aludwiko left a comment

johanandren Oct 12, 2023

franciscolopezsancho Oct 13, 2023

franciscolopezsancho Oct 13, 2023

franciscolopezsancho Oct 13, 2023

johanandren left a comment

		@@ -52,18 +50,27 @@ public abstract class DockerIntegrationTest {

		private KalixSpringApplication kalixSpringApplication;

		public DockerIntegrationTest(ApplicationContext applicationContext) {

feat: adding traces to UF components #1763

feat: adding traces to UF components #1763

Conversation

franciscolopezsancho commented Aug 10, 2023

franciscolopezsancho commented Sep 12, 2023

octonato left a comment

Choose a reason for hiding this comment

franciscolopezsancho commented Sep 17, 2023

johanandren left a comment

Choose a reason for hiding this comment

aludwiko left a comment

Choose a reason for hiding this comment

johanandren Oct 12, 2023

Choose a reason for hiding this comment

franciscolopezsancho Oct 13, 2023

Choose a reason for hiding this comment

franciscolopezsancho Oct 13, 2023

Choose a reason for hiding this comment

franciscolopezsancho Oct 13, 2023

Choose a reason for hiding this comment

johanandren left a comment

Choose a reason for hiding this comment