Skip to content

apache camel 2.20.x instrumentation#1397

Merged
iNikem merged 13 commits into
masterfrom
unknown repository
Nov 3, 2020
Merged

apache camel 2.20.x instrumentation#1397
iNikem merged 13 commits into
masterfrom
unknown repository

Conversation

@ghost

@ghost ghost commented Oct 15, 2020

Copy link
Copy Markdown

Fixes #1356

Instrumentation for Apache Camel 2.20.x
Apache Camel provides OTEL instrumentation (https://camel.apache.org/components/latest/others/opentelemetry.html) but for Camel >= 3.5. For customers using older versions that don't want / can't move to 3.5 or newer, a separate instrumentation library is provided.

NOTE
Large portions of the code has been adapted from existing Apache Camel OpenTracing library. In order to honour the license and work of the original Authors, I've added following header:

/*
 * Includes work from:
 * Copyright Apache Camel Authors
 * SPDX-License-Identifier: Apache-2.0
 */

to all files consisting at least some code from the library.
The header was put under "package" in order to pass spotless:license plugin check.
I'm not entirely sure that the header is 100% compatible with Apache License 2.0 of the original code (see https://github.com/apache/camel/tree/camel-2.20.0/components/camel-opentracing). Therefore during review please have a look at this as well.

@mateuszrzeszutek mateuszrzeszutek left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two more questions:

  • Have you tested it on a 3.X camel project? Can we be sure that this instrumentation won't interfere with Camel's OTel instrumentation?
  • Can you add some multithreaded tests (e.g. multicast & aggregate)? To make sure that context is propagated correctly

Comment thread instrumentation/apache-camel-2.20/apache-camel-2.20.gradle Outdated
@ghost

ghost commented Oct 20, 2020

Copy link
Copy Markdown
Author

@mateuszrzeszutek added new tests (multithreaded and simple internal direct). Also - added logic to prevent wrong CLIENT spas with all internal components. Please have another look :)

@jkwatson

Copy link
Copy Markdown
Contributor

I'm not a primary reviewer for this, but I wonder how it is possible to effectively review a 2222 line PR. Can this be broken up into a series of smaller PRs to make it easier for maintainers to review?

@ghost ghost requested review from anuraaga and mateuszrzeszutek October 21, 2020 09:16
@ghost

ghost commented Oct 22, 2020

Copy link
Copy Markdown
Author

@iNikem @anuraaga @trask @tylerbenson please have a look, looking into closing this PR this week. Thanks in advance!

@iNikem

iNikem commented Oct 22, 2020

Copy link
Copy Markdown
Contributor

Honestly, I am waiting for @mateuszrzeszutek approval :) He has done such thorough first review.

@ghost

ghost commented Oct 22, 2020

Copy link
Copy Markdown
Author

@iNikem yep, I'm nagging him on a daily basis ;)

@ghost

ghost commented Oct 28, 2020

Copy link
Copy Markdown
Author

@mateuszrzeszutek pls have another look ;)

Comment thread instrumentation/apache-camel-2.20/apache-camel-2.20.gradle
Comment thread instrumentation/apache-camel-2.20/apache-camel-2.20.gradle
@iNikem iNikem merged commit 049358e into open-telemetry:master Nov 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Apache Camel 2.20.x instrumentation

5 participants