Added initial commit of camunda instrumentation

[cb645j]

related to issue #12122

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: cb645j / name: Boslet, Cory (cb645j) (9fb1aed, 4fbdbf8, 193cd63, bdd9ea1, 26df3f1, da9e851, 38ff7f0, c5718bc, d7f179f, b267316, fd44b57, cc90160, 3f9833a, 979c377, c6aa4c2, 308f6a8, fb19def, 80b0e67, 7fcfa7d, d60eaf3, 6e797eb, f938b85, 7b268c0, 75f3fdc, 628d839, 3277d7e, e484aa7, b44bc9d, a631796, bcf17bf, 0cd3185, 0a2a414, 1f4d357, 4355c63, d0d2c6c, 4f13543, 9e5df91, 5e151e3, a3fd09c, 2aa9616, 096fb01, 305c58a, bb18387, b6d1b32, 35ae48e)
• ✅ login: cb645j / name: cb645j (0c9ac2d, 62e9317, 78bc660)
• ✅ login: otelbot[bot] / name: otelbot (7aa48b4, 1298c3f, ad4f524)

[cb645j]

@robododge

[cb645j]

@laurit can you please review this

[laurit]

@laurit can you please review this

There are no tests. You didn't add your module to settings.gradle.kts so it is unknown whether it compiles.

[robododge]

Thanks @cb645j for starting this submission. Yes @laurit is correct, this needs to be integrated with settings.gradle and the full test suite.

A few questions.

There a lot of files in this PR, I counted 7 distinct TypeInstrumentations for the following Camunda classes:

1. org.camunda.bpm.engine.impl.bpmn.behavior.CallableElementActivityBehavior
2. org.camunda.bpm.engine.impl.bpmn.behavior.TerminateEndEventActivityBehavior
3. org.camunda.bpm.engine.impl.bpmn.behavior.ExternalTaskActivityBehavior
4. org.camunda.bpm.engine.impl.bpmn.behavior.TaskActivityBehavior
5. org.camunda.bpm.engine.impl.jobexecutor.AsyncContinuationJobHandler
6. org.camunda.bpm.engine.runtime.ProcessInstantiationBuilder
7. org.camunda.bpm.client.topic.impl.TopicSubscriptionManager

First question: Can you merge the instrumentation for the like-components?... I.e. All the "Behavior" components go through a single TypeInstrumentation?

Second question: What about the version, does this handle only Camunda 7? or does this handle camunda 7 and greater? What version is targeted?

Thanks

[cb645j]

Thanks @cb645j for starting this submission. Yes @laurit is correct, this needs to be integrated with settings.gradle and the full test suite.

A few questions.

There a lot of files in this PR, I counted 7 distinct TypeInstrumentations for the following Camunda classes:

1. org.camunda.bpm.engine.impl.bpmn.behavior.CallableElementActivityBehavior

2. org.camunda.bpm.engine.impl.bpmn.behavior.TerminateEndEventActivityBehavior

3. org.camunda.bpm.engine.impl.bpmn.behavior.ExternalTaskActivityBehavior

4. org.camunda.bpm.engine.impl.bpmn.behavior.TaskActivityBehavior

5. org.camunda.bpm.engine.impl.jobexecutor.AsyncContinuationJobHandler

6. org.camunda.bpm.engine.runtime.ProcessInstantiationBuilder

7. org.camunda.bpm.client.topic.impl.TopicSubscriptionManager

First question: Can you merge the instrumentation for the like-components?... I.e. All the "Behavior" components go through a single TypeInstrumentation?

Second question: What about the version, does this handle only Camunda 7? or does this handle camunda 7 and greater? What version is targeted?

Thanks

Thanks Steve. I think I can probably merge a few of those typeinstrumentations into single classes, i will take a look! Yes, i tested with versions 7 and 8 but I believe it should handle anything above 7 as I did not see anything in later versions that would break so i left the max version undefined in the settings muzzle.pass, I think thats what your suppose to do when it supports version x and above? Camunda noticed that will no longer support versions 6 and under so I did not worry about those ones.

[cb645j]

@laurit can you please review this

There are no tests. You didn't add your module to settings.gradle.kts so it is unknown whether it compiles.

Opps, I add it as a module but forgot to check that file in, ill update.. Ill work on adding unit test. thanks

[robododge]

Thanks Steve. I think I can probably merge a few of those typeinstrumentations into single classes, i will take a look! Yes, i tested with versions 7 and 8 but I believe it should handle anything above 7 as I did not see anything in later versions that would break so i left the max version undefined in the settings muzzle.pass, I think thats what your suppose to do when it supports version x and above? Camunda noticed that will no longer support versions 6 and under so I did not worry about those ones.

@cb645j Cory, that v7 and above support is great.

Recommendation 1: versioning: I'd recommend moving everything into directory that states "camunda-7", which covers all of 7 and possibly 8 if the right classes. This version 7, 8 support can be is enforced in the override of the classLoaderMatcher() method of InstrumentationModule, you want to choose a class that is really unique to version 7 at least. Also, you muzzle config will want to drop back down to the 7.0 or the lowest level of version 7 you aim to support.

Recommendation 2: folder structure: I'd recommend restructurctuing the entire thing to adhere to the Folder Structure Style Guide. where you have 3 main directories, javaagent, library and testing directories.

Recommendation 3: testing:
Also, maybe you want to take a look at the Contibuting.md, especially the sections regarding testing. You want the classes that kick off your instrumentation in the format that the Opentelemetry-Java-Instrumentation project expects, this means having a *Tracing and *TracingBuilder entry point. Detailed Testing section.

[laurit]

Recommendation 2: folder structure: I'd recommend restructurctuing the entire thing to adhere to the Folder Structure Style Guide. where you have 3 main directories, javaagent, library and testing directories.

This only makes sense if you are going to have both library and agent instrumentations. If you are going to have only the javaagent instrumentation only that module is enough. No need to have a separate testing module.

Recommendation 3: testing: Also, maybe you want to take a look at the Contibuting.md, especially the sections regarding testing. You want the classes that kick off your instrumentation in the format that the Opentelemetry-Java-Instrumentation project expects, this means having a *Tracing and *TracingBuilder entry point. Detailed Testing section.

Again if you are going to have only the agent instrumentation then you don't really need to create abstract test classes that have only 1 implementaion.

[robododge]

Thanks @laurit for clearing up the Library vs Agent instrumentation. I was assuming that the rule was you need to create both as that was what I was required to do for Jetty-Httpclient-9.2. So, now, if we don't feel the necessity for a Library-based instrumentation, then we can choose to simply implement Javagent only?

Thanks

[robododge]

So, now, if we don't feel the necessity for a Library-based instrumentation, then we can choose to simply implement Javagent only?

I took another look at the Writing Instumentation doc. It seems the rule of thumb is if you have target classes that can support Library-style instrumentation, then you should provide the library feature as an option.

[laurit]

Thanks @laurit for clearing up the Library vs Agent instrumentation. I was assuming that the rule was you need to create both as that was what I was required to do for Jetty-Httpclient-9.2. So, now, if we don't feel the necessity for a Library-based instrumentation, then we can choose to simply implement Javagent only?

Thanks

If a library instrumentation can be built we definitely would like to have one, but it is not always possible to write one.

[grandemtn]

Thank you for working on this! This will be a great help to many of my projects as we use SpringBoot and Camunda 7 extensively. Recently we have started integrating with Datadog.

Request: can you include the businessKey in the CamundaCommonRequest and code that populates it? That would help us identify the traces/spans.
Ideally there's another variable in our executions that identifies the global transaction id that would be wonderful to have on the spans as well, but it's specific to our implementation. Could you provide a config value with the name of the execution variable to include as well? Or some way to pull another variable out of the execution and include it in the span attributes?

I am new to OpenTelemetry, Observability and Instrumentation in general. I have quite a lot of experience with Java, Spring and Camunda. If there is something I could to do help with this, please let me know!

[cb645j]

Thank you for working on this! This will be a great help to many of my projects as we use SpringBoot and Camunda 7 extensively. Recently we have started integrating with Datadog.

Request: can you include the businessKey in the CamundaCommonRequest and code that populates it? That would help us identify the traces/spans. Ideally there's another variable in our executions that identifies the global transaction id that would be wonderful to have on the spans as well, but it's specific to our implementation. Could you provide a config value with the name of the execution variable to include as well? Or some way to pull another variable out of the execution and include it in the span attributes?

I am new to OpenTelemetry, Observability and Instrumentation in general. I have quite a lot of experience with Java, Spring and Camunda. If there is something I could to do help with this, please let me know!

Yes, I should be able to add the businesskey. As far as implementation specific variables, i did plan to add something exactly as your describing - the ability for the user to identify additional execution variables they want to be captured (via a comma separated config property), however I plan to make this change as a separate enhancement after I get this PR merged. I would like to just get this basic implementation in before making additional enhancements, but i can add the businesskey as part of this PR since that is a global camunda property and will be easy to do.

[laurit]

is this needed?

[cb645j]

no, will remove.. instrumentations enabled by default, correct?

[jaydeluca]

yes

[robbydf]

Hi @cb645j . By any chance, do you have an extension you could provide while this PR is ongoing?

[cb645j]

Hi @cb645j . By any chance, do you have an extension you could provide while this PR is ongoing?

i do but unfortunately its not opensource, im still working im getting this merged so its available to all. Feel free to contribute to this PR, i can give you permissions to commit to my repo/branch if needed.

[cb645j]

@laurit Thanks for reviewing. I have made changes to address your comments, can you please review, approve, and merge. Thank you.

[otelbot-java-instrumentation]

🔧 The result from spotlessApply was committed to the PR branch.

[otelbot-java-instrumentation]

🔧 The result from spotlessApply was committed to the PR branch.

[otelbot-java-instrumentation]

🔧 The result from spotlessApply was committed to the PR branch.

[cb645j]

@laurit @jaydeluca can you please review. ive made updates to address every comment

[robenie-cwt]

Tagging @zeitlinger @jack-berg @breedx-splk @jeanbisutti @jkwatson @JonasKunz @steverao @SylvainJuge and @trask as well for the review