CAMEL-15339 Opentelemetry component by rubenvp8510 · Pull Request #4016 · apache/camel

rubenvp8510 · 2020-07-18T21:22:04Z

The intention of this PR is to add opentelemetry support to camel,

This initial PR will do the same as camel-opentracing component, but will use opentelemetry to do it. I had to refactor camel-opentracing in order to reuse span decorators. In order to do that I created a new component camel-tracing which contains all common interfaces used by SpanDecorator, and a general Tracer abstract class . Both implementations opentracing and opentelemetry should extend Tracer and implement the abstract methods, they should also implement the SpanAdapter interface ,the interface used by the decorators to add tags, labels etc.. to the Span.

Not sure if this is the right approach If someone could give me more feedback on this will be appreciated. I would like o hear more options.

oscerd

Please run with the sourcecheck profile enabled because the indentation in parent pom seems to be wrong and in the components you'll get a bunch of errors to fix in terms of codestyle.

Also, what is the purpose of this PR? I see a camel-tracing component introduced. This seems to be a big change and refactoring so it probably deserve a JIRA issue with some explanation of what is the plan and what will be in the common module and what not.

Thanks.

oscerd · 2020-07-20T06:34:06Z

+    <dependency>
+      <groupId>io.grpc</groupId>
+      <artifactId>grpc-netty-shaded</artifactId>
+      <version>1.28.0</version>


you should use the version from parent POM

oscerd · 2020-07-20T06:34:20Z

+    <dependency>
+      <groupId>io.opentelemetry</groupId>
+      <artifactId>opentelemetry-sdk</artifactId>
+      <version>0.6.0</version>


It should be declared in the parent POM

oscerd · 2020-07-20T06:34:45Z

+ */
+package org.apache.camel.opentelemetry;
+
+import java.util.*;


Please use the explicit imports, no * operator

oscerd · 2020-07-20T06:35:57Z

@@ -24,6 +24,7 @@
 import org.apache.camel.spi.IdAware;


This will probably create conflicts with this #4001 once we merge it

I see :( I'll fix those conflicts.

oscerd · 2020-07-20T06:36:46Z

-import java.util.List;
-import java.util.Map;
-import java.util.Set;
+import java.net.*;


Please use the explicit imports.

rubenvp8510 · 2020-07-20T13:34:40Z

Please run with the sourcecheck profile enabled because the indentation in parent pom seems to be wrong and in the components you'll get a bunch of errors to fix in terms of codestyle.

Also, what is the purpose of this PR? I see a camel-tracing component introduced. This seems to be a big change and refactoring so it probably deserve a JIRA issue with some explanation of what is the plan and what will be in the common module and what not.

Thanks.

Hi, Thanks for the review, the purpose is to support opentelemetry, doing the same as opentracing does but using opentelemetry instead.

The purpose of the refactorization is for reuse the span decorators. I'll check the code style, but I ran the mvn clean install -Psourcecheck locally and it passed. :/

Signed-off-by: Ruben <ruben.vp8510@gmail.com>

oscerd

LGTM, @davsclaus can you have a look?

davsclaus

Only a few comments that we can also fix in a 2nd PR

davsclaus · 2020-07-28T07:13:37Z

+    <dependency>
+      <groupId>io.opentelemetry</groupId>
+      <artifactId>opentelemetry-exporters-inmemory</artifactId>
+      <version>0.6.0</version>


Use version placeholder

davsclaus · 2020-07-28T07:14:29Z

+
+    @Override public void log(Map<String, String> fields) {
+        span.addEvent(getEventNameFromFields(fields), convertToAttributes(fields));
+


Remove empty line

davsclaus · 2020-07-28T07:15:57Z

+
+public class OpenTelemetryGetter implements HttpTextFormat.Getter<ExtractAdapter> {
+
+    @Nullable @Override public String get(ExtractAdapter adapter, String key) {


We do not use Nullable annotations in camel (yet)

davsclaus · 2020-07-28T07:19:50Z

+        return false;
+    }
+
+    private final class TracingEventNotifier extends EventNotifierSupport {


In the logging below change OpenTracing to Tracing

Signed-off-by: Ruben <ruben.vp8510@gmail.com>

davsclaus · 2020-07-29T05:23:42Z

After this PR we should likely get camel-zipkin to use camel-tracing as well.
And then we need to update the camel karaf features also

oscerd · 2020-07-29T05:34:14Z

I'm going to merge today. I'm not completely sure it will possible to do the job for Zipkin anyway.

oscerd · 2020-07-29T05:51:37Z

I guess we'll have to regen also the spring-boot starters related to this.

jam01 · 2020-07-29T05:57:54Z

There are some changes from #4044 that'll have to reworked into the new tracing component, I believe.

oscerd · 2020-07-29T05:59:13Z

@jam01 I do believe we need to merge this PR asap, so you'll be able to align, let me merge it.

oscerd · 2020-07-29T06:10:55Z

Thanks for this @rubenvp8510

oscerd · 2020-07-29T06:45:31Z

@jam01 this has been merged, I'll go ahead with Karaf stuff and Spring Boot, but you can already had your PR to camel-tracing

rubenvp8510 force-pushed the opentelemetry branch 3 times, most recently from 2e85ed4 to c225d92 Compare July 19, 2020 01:01

oscerd requested changes Jul 20, 2020

View reviewed changes

rubenvp8510 force-pushed the opentelemetry branch 9 times, most recently from 0860f26 to 17eee21 Compare July 25, 2020 00:35

Refactor span decorators to be used in opentelemetry/opentracing

f6cd295

Signed-off-by: Ruben <ruben.vp8510@gmail.com>

rubenvp8510 force-pushed the opentelemetry branch 3 times, most recently from 1f04f1a to 6b4e2bc Compare July 25, 2020 04:37

Unify tracing class

819ccdc

Signed-off-by: Ruben <ruben.vp8510@gmail.com>

rubenvp8510 force-pushed the opentelemetry branch from 6b4e2bc to ac74e28 Compare July 27, 2020 04:59

opentracing basic tests

3bae005

Signed-off-by: Ruben <ruben.vp8510@gmail.com>

rubenvp8510 force-pushed the opentelemetry branch from ac74e28 to 3bae005 Compare July 27, 2020 13:40

rubenvp8510 changed the title ~~[WIP] opentelemetry support~~ Opentelemetry support Jul 27, 2020

rubenvp8510 changed the title ~~Opentelemetry support~~ [CAMEL-15339] Opentelemetry component Jul 27, 2020

rubenvp8510 requested a review from oscerd July 27, 2020 16:20

rubenvp8510 changed the title ~~[CAMEL-15339] Opentelemetry component~~ CAMEL-15339 Opentelemetry component Jul 28, 2020

oscerd approved these changes Jul 28, 2020

View reviewed changes

davsclaus approved these changes Jul 28, 2020

View reviewed changes

Fix tracing log messages, some improvments on code style

0e09123

Signed-off-by: Ruben <ruben.vp8510@gmail.com>

oscerd merged commit ff6d52b into apache:master Jul 29, 2020


		@Override public void log(Map<String, String> fields) {
		span.addEvent(getEventNameFromFields(fields), convertToAttributes(fields));


		public class OpenTelemetryGetter implements HttpTextFormat.Getter<ExtractAdapter> {

		@Nullable @Override public String get(ExtractAdapter adapter, String key) {

Conversation

rubenvp8510 commented Jul 18, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

oscerd left a comment

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!

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!

rubenvp8510 commented Jul 20, 2020

Uh oh!

oscerd left a comment

Choose a reason for hiding this comment

Uh oh!

davsclaus left a comment

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!

Choose a reason for hiding this comment

Uh oh!

davsclaus commented Jul 29, 2020

Uh oh!

oscerd commented Jul 29, 2020

Uh oh!

oscerd commented Jul 29, 2020

Uh oh!

jam01 commented Jul 29, 2020

Uh oh!

oscerd commented Jul 29, 2020

Uh oh!

oscerd commented Jul 29, 2020

Uh oh!

oscerd commented Jul 29, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

rubenvp8510 commented Jul 18, 2020 •

edited

Loading