Trying to extract codegen plugin #3521

iNikem · 2021-07-07T20:05:17Z

No description provided.

Add publishing plugin

buildSrc/build.gradle.kts

buildSrc/src/main/kotlin/io.opentelemetry.instrumentation.javaagent-instrumentation.gradle.kts

...in/java/io/opentelemetry/javaagent/muzzle/generation/collector/AdviceClassNameCollector.java

...rc/main/java/io/opentelemetry/javaagent/extension/instrumentation/InstrumentationModule.java

muzzle/build.gradle.kts

iNikem · 2021-07-09T07:05:19Z

Asking for specific feedback:

Name of the plugin module. Currently javaagent-muzzle-generation. Ok/Nok/better ideas?
Id of the plugin. Currently io.opentelemetry.instrumentation.javaagent-codegen. Ok/Nok/better ideas?

anuraaga · 2021-07-09T07:22:16Z

javaagent-muzzle-generation/build.gradle.kts

+group = "io.opentelemetry.instrumentation.gradle"
+
+dependencies {
+  implementation("com.google.guava:guava")


Sorry had a pending comment but forgot to hit submit. On the topic of a codegen configuration

I think we may still want the configuration

To add less to Gradle classpath. If we add too much it can cause dependency hell, a bit worried about Guava

To be able to substitute in the project dependencies in our project, not sure we'd have any way of doing it for a plugin transitive dependency which would be resolved way too early for a substitution to kick in I think.

I guess 1 could be resolved with shading anyways though. But 2) seems important and a requirement to allow using an older version of the plugin even as we work on the javaagent-extension-api.

At the same time self-contained plugin is good for external users. They don't need to know about "codegen" dependency and that something has to be added to it.

Yeah they wouldn't need to with the configuration either since we configure it with defaults here. Before this PR our repo is substituting to use our project dependencies but it's not required to care otherwise.

I am afraid I don't quite understand what do you suggest? Do you mean to create and use codegen configuration already here, right in the module? Or how?

I propose using the same pattern we have

https://github.com/open-telemetry/opentelemetry-java-instrumentation/blob/main/buildSrc/src/main/kotlin/io.opentelemetry.instrumentation.javaagent-testing.gradle.kts#L33

This as all the io.openteletry.* ones are meant to be published for downstream. Though most will get this one transitively via io.opentelemetry.instrumentation.javaagent-instrumentation.

The configuration is set here when the plugin is included so most users won't need to worry about it.

Also makes me realize this project shouldn't be called muzzle or codegen or anything - let's call it gradle-plugins. We have a few plugins to publish.

Also makes me realize this project shouldn't be called muzzle or codegen or anything - let's call it gradle-plugins. We have a few plugins to publish.

Which ones? I am fine with publishing muzzle generation and checking as one plugin. But I am not sure we should bundle all our plugins into one artifact?

I propose using the same pattern we have

https://github.com/open-telemetry/opentelemetry-java-instrumentation/blob/main/buildSrc/src/main/kotlin/io.opentelemetry.instrumentation.javaagent-testing.gradle.kts#L33

This as all the io.openteletry.* ones are meant to be published for downstream. Though most will get this one transitively via io.opentelemetry.instrumentation.javaagent-instrumentation.

The configuration is set here when the plugin is included so most users won't need to worry about it.

Do I understand you correctly?

Published io.opentelemetry.instrumentation.javaagent-codegen plugin does not have any declared runtime dependencies

Actual dependencies, such as guava and javaagent-extension-api are added by io.opentelemetry.instrumentation.javaagent-instrumentation plugin

End users are supposed to use io.opentelemetry.instrumentation.javaagent-instrumentation plugin when it gets published

Yeah that understanding sounds correct to me.

But I am not sure we should bundle all our plugins into one artifact?

I don't think there's any real downside - anyways we'll publish to Gradle Plugin Portal so users just add the plugin to their project and don't care how the artifact is structured. So it allows us to keep all plugin publishing type of logic together.

First, we don't actually HAVE to publish to Gradle Plugin Portal. Plugins can be consumed just from Maven Central as well.

Second, ok, let's give it a try...

anuraaga · 2021-07-09T07:22:57Z

javaagent-muzzle-generation/build.gradle.kts

@@ -0,0 +1,22 @@
+plugins {


Do we want the word muzzle in these projects? Assuming muzzle refers to our safety mechanism for applying instrumentation across versions, a lot of the codegen is independent of muzzle

This plugin generates bodies for InstrumentationModule methods with word muzzle in method names. So currently it is certainly muzzle-related.

anuraaga · 2021-07-09T07:23:50Z

javaagent-muzzle-generation/build.gradle.kts

+  website = "https://opentelemetry.io"
+  vcsUrl = "https://github.com/open-telemetry/opentelemetry-java-instrumentation"
+  tags = listOf("opentelemetry", "instrumentation")
+}


We'll need that plugins snippet from my draft PR when publishing to portal so may as well add it now right?

Which PR? :) Sorry, you are too laconic for me recently :(

Sorry for my smartphone replies, somehow GitHub app still doesn't support linking to lines of code. And now that I look for it I can't find it in the PR I sent, that's definitely too laconic...

gradlePlugin { plugins { named("io.opentelemetry.instrumentation.javaagent-codegen") { displayName = "<short displayable name for plugin>" description = "<Good human-readable description of what your plugin is about>" } } }

https://docs.gradle.org/current/userguide/publishing_gradle_plugins.html#configure_the_plugin_publishing_plugin

One thing I found is for precompiled script plugin, we leave out id and implementationClass and it seems to work ok

...gent-muzzle-generation/src/main/java/io/opentelemetry/javaagent/muzzle/generation/Utils.java

…ublished

iNikem · 2021-07-12T08:43:08Z

For the reference: here is an example of using this new plugin in an external project: https://github.com/signalfx/splunk-otel-java/compare/muzzle-codegen?expand=1. Only one module was verified to work for now.

I am concerned about using codegen configuration in that module's build script.

anuraaga

Just nits, thanks!

buildSrc/src/main/kotlin/io.opentelemetry.instrumentation.javaagent-instrumentation.gradle.kts

buildSrc/src/main/kotlin/io.opentelemetry.instrumentation.javaagent-testing.gradle.kts

...s/src/main/java/io/opentelemetry/javaagent/muzzle/generation/MuzzleCodeGenerationPlugin.java

...-plugins/src/main/java/io/opentelemetry/javaagent/muzzle/generation/MuzzleCodeGenerator.java

...e-plugins/src/main/java/io/opentelemetry/javaagent/muzzle/generation/ReferenceCollector.java

...ain/java/io/opentelemetry/javaagent/tooling/muzzle/collector/MuzzleCompilationException.java

mateuszrzeszutek · 2021-07-14T09:42:36Z

buildSrc/src/main/kotlin/io.opentelemetry.instrumentation.javaagent-instrumentation.gradle.kts

 }

 dependencies {
  add("muzzleBootstrap", "io.opentelemetry.javaagent:opentelemetry-javaagent-bootstrap")
  add("muzzleTooling", "io.opentelemetry.javaagent:opentelemetry-javaagent-tooling")
  add("muzzleTooling", "io.opentelemetry.javaagent:opentelemetry-javaagent-extension-api")
+
+  add("codegen", "io.opentelemetry.javaagent:opentelemetry-javaagent-tooling")


Is tooling needed for codegen?

Apparently not right now...

mateuszrzeszutek · 2021-07-14T09:43:20Z

examples/extension/build.gradle

@@ -11,6 +11,8 @@ plugins {
  See https://imperceptiblethoughts.com/shadow/ for more details about Shadow plugin.
   */
  id "com.github.johnrengelman.shadow" version "6.1.0"
+
+  id "io.opentelemetry.instrumentation.javaagent-codegen" version "0.1.0"


Is 0.1.0 some temporary released version? Are we going to version gradle plugins the same way we do all other artficats?

Currently this module is totally independent from the main build. I don't have a strong opinion on its versioning atm

iNikem and others added 3 commits July 7, 2021 23:04

Trying to extract codegen plugin

ac70317

Add publishing plugin

82f1057

Merge pull request #101 from anuraaga/extract-codegen-anuraag

a055945

Add publishing plugin

anuraaga reviewed Jul 8, 2021

View reviewed changes

buildSrc/build.gradle.kts Outdated Show resolved Hide resolved

iNikem added 5 commits July 8, 2021 09:40

Polish?

96e0b6e

Proper plugin application

5fa80ee

Muzzle code generation moved to a separate plugin

1a4b409

Fix dependencies

b2eff47

Merge remote-tracking branch 'upstream/main' into extract-codegen

6f8a7b6

mateuszrzeszutek reviewed Jul 8, 2021

View reviewed changes

Renamed plugin module to javaagent-muzzle-generation

c5fff32

iNikem marked this pull request as ready for review July 9, 2021 07:05

iNikem requested review from jkwatson, laurit, pavolloffay, trask and tylerbenson as code owners July 9, 2021 07:05

anuraaga reviewed Jul 9, 2021

View reviewed changes

iNikem added 2 commits July 9, 2021 10:48

Spotless

5417c25

Restored old MuzzleCodeGenerationPlugin to use until the new one is p…

026942c

…ublished

iNikem added 3 commits July 12, 2021 15:48

Renamed plugin module

5b70f42

The simplest dependency management possible

987cc83

Polish

fddaa44

anuraaga approved these changes Jul 14, 2021

View reviewed changes

iNikem added 2 commits July 14, 2021 11:39

Polish

9b087bf

Merge remote-tracking branch 'upstream/main' into extract-codegen

5bfd291

mateuszrzeszutek approved these changes Jul 14, 2021

View reviewed changes

Removed unneeded dep

88eb2f7

iNikem merged commit cbfd7e1 into open-telemetry:main Jul 14, 2021

iNikem deleted the extract-codegen branch July 14, 2021 14:08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Trying to extract codegen plugin #3521

Trying to extract codegen plugin #3521

iNikem commented Jul 7, 2021

iNikem commented Jul 9, 2021

anuraaga Jul 9, 2021

iNikem Jul 9, 2021

anuraaga Jul 9, 2021

iNikem Jul 9, 2021

anuraaga Jul 9, 2021

anuraaga Jul 9, 2021

iNikem Jul 12, 2021

iNikem Jul 12, 2021

anuraaga Jul 12, 2021

iNikem Jul 12, 2021

anuraaga Jul 9, 2021

iNikem Jul 9, 2021

anuraaga Jul 9, 2021

iNikem Jul 12, 2021

anuraaga Jul 12, 2021

iNikem commented Jul 12, 2021

anuraaga left a comment

mateuszrzeszutek Jul 14, 2021

iNikem Jul 14, 2021

mateuszrzeszutek Jul 14, 2021

iNikem Jul 14, 2021

Trying to extract codegen plugin #3521

Trying to extract codegen plugin #3521

Conversation

iNikem commented Jul 7, 2021

iNikem commented Jul 9, 2021

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

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

iNikem commented Jul 12, 2021

anuraaga left a 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