[maven-extension] Capture details on mojo goal executions: `deploy:deploy`, `spring-boot:build-image`, `jib:build`, `snyk:test`, `snyk:monitor` #146

cyrille-leclerc · 2021-12-06T13:58:52Z

Description:

Capture details on mojo goal executions: deploy:deploy, spring-boot:build-image, jib:build, snyk:test, and snyk:monitor

Existing Issue(s):

None

Testing:

Introduced unit tests.

Documentation:

See the updated README.md

Outstanding items:

Span Attributes introduced in this PR

Span attributes introduced in this PR.

Key namespaces:

maven.*: anything related to Maven
- maven.build.*: anything related to the artifacts produced by Maven builds
  - maven.build.container: producing docker/container images
    - maven.build.container.image.*: anything related to container images produced by Maven builds
    - maven.build.container.registry.*: attributes to describe container registry to which produced docker images are uploaded., describe the urlof the registry and the username used for authentication.
  - maven.build.repository: uploading artifacts to a Maven repository
- maven.execution.*: anything related to the Maven execution (e.g. the goal)
- maven.plugin.*: anything related to the Maven plugin that is being executed
- maven.project.*: anything related to the project/module that is building

Noteworthy: we capture the username used to upload to container registries and Maven repositories to help troubleshooting

Span attribute	Type	Description
`maven.build.container.image.name`	string	Name of the produced Docker image
`maven.build.container.image.tags`	string[]	Tags of the produced Docker image
`maven.build.container.registry.auth.username`	string	Username used to upload the image to the container registry.
`maven.build.container.registry.url`	string	URL of the container registry to which this image is uploaded.
`maven.build.repository.auth.username`	string	Username used to upload artifacts to the Maven repository. See Maven POM reference / Repository
`maven.build.repository.id`	string	ID of the Maven repository to which the artifact is deployed. See Maven POM reference / Repository
`maven.build.repository.url`	string	URL of the Maven repository to which the artifact is deployed. See Maven POM reference / Repository
`maven.execution.goal`	string	Goal that is being executed
`maven.execution.id`	string	ID of the execution
`maven.execution.lifecyclePhase`	string	Lifecycle phase to which belong the execution
`maven.plugin.artifactId`	string	Artifact ID of the Maven plugin on which the Maven goal is executed
`maven.plugin.groupId`	string	Group ID of the Maven plugin on which the Maven goal is executed
`maven.plugin.version`	string	Version of the Maven plugin on which the Maven goal is executed
`maven.project.artifactId`	string	Artifact ID of the Maven project on which the Maven goal is executed
`maven.project.groupId`	string	Group ID of the Maven project on which the Maven goal is executed
`maven.project.version`	string	Version of the Maven project on which the Maven goal is executed

…uild-image

…jo-goal-executions

maven-extension/src/main/java/io/opentelemetry/maven/handler/DeployDeployHandler.java

maven-extension/src/main/java/io/opentelemetry/maven/OtelExecutionListener.java

maven-extension/src/main/java/io/opentelemetry/maven/handler/MojoGoalExecutionHandler.java

maven-extension/README.md

cyrille-leclerc · 2021-12-10T16:30:58Z

@kenfinnigan the visibility OTel tracing brings on the flow of the build is amazing! It can greatly help teams optimize their builds.

I discover that even for pretty simple projects, it's easy to wrongly configure the chain of goals of a Maven build and have redundant executions of costly goals.

Here is an example below of a build just combining a Spring Boot app with Snyk checks, a deploy to a Maven repo, and and the publishing as a docker image.
I got some pom.xml fragments from snyk.io and Spring Boot web site to assemble a mvn deploy and the goals surefire:test, snyk:test are executed twice impacting seriously the duration (pom.xml).

cyrille-leclerc · 2021-12-15T22:11:51Z

@kenfinnigan @trask I have added in the description of this PR all the introduced span attributes with an explanation on the used namespaces (container.registry.image.* and maven.*) including the maven.build.* we have just discussed.
The detailed description of all the attribute per Maven Plugin Goal execution is available on the updated README.md.

I have implemented the enrichment for 5 plugin goals in order to have a broad set of use cases.

Even if @trask suggested to subdivide the maven.build.* namespace per plugin that build things, I tried to use a common namespace for maven.build.container.image.* for Docker images produced either by the Spring Boot Maven Plugin or by the Google Jib Maven Plugin and IMO it works well.

I can easily implement @trask's suggestion with maven.springboot.build.container.image.* and maven.googlejib.build.container.image.* if we prefer.

trask · 2021-12-15T22:33:24Z

I tried to use a common namespace for maven.build.container.image.* for Docker images produced either by the Spring Boot Maven Plugin or by the Google Jib Maven Plugin and IMO it works well.

👍

on first glance, it may make sense to move container.registry.url and container.registry.username to maven.build.container.registry.*, which matches maven.build.container.image.*, and avoids some confusion with container.* resource attributes spec

...extension/src/main/java/io/opentelemetry/maven/handler/AbstractMojoGoalExecutionHandler.java

anuraaga · 2021-12-16T04:54:49Z

...src/main/resources/META-INF/services/io.opentelemetry.maven.handler.MojoGoalExecutionHandler

@@ -0,0 +1,5 @@
+io.opentelemetry.maven.handler.GoogleJibBuildHandler


What do you think about avoiding SPI for our built in handlers? Then we could make them non-public too, just implementation details.

I introduced the SPI after discussing with @kenfinnigan . I see value in allowing Maven plugin authors to add instrumentation for the their plugins (plugin authors being vendors, oss projects or Maven practitioners).

What solution do you have in mind as an alternative to the SPI?
Before implementing the SPI, I used a hardcoded list of handlers, could it be what you had in mind?

Ah I had meant a list of built in components that is appended to with SPI. I think it satisfies both itches.

Done through d393bea . Please review

maven-extension/src/main/java/io/opentelemetry/maven/MavenUtils.java

maven-extension/src/main/java/io/opentelemetry/maven/OtelExecutionListener.java

anuraaga · 2021-12-16T05:01:16Z

maven-extension/src/main/java/io/opentelemetry/maven/OtelExecutionListener.java

-            .startSpan();
+                mojoExecution.getLifecyclePhase());
+    //  enrich spans with MojoGoalExecutionHandler
+    Optional<MojoGoalExecutionHandler> handler =


We don't use Optional much in OTel codebases in favor of Nullable. I just noticed I've been forgetting to add nullness checking to this repo like we have in SDK repo, will add that. Can you look into reducing Optional for more consistency with SDK repo? I don't think the usages here are all that different from null checks

I like the principle "When in Rome, do as the Romans do" :-). I'll replace the usage of Optional by using @Nullable

Migration from Optional to @Nullable done. Please verify

maven-extension/src/main/java/io/opentelemetry/maven/handler/MavenDeployHandler.java

cyrille-leclerc · 2021-12-16T11:06:02Z

I tried to use a common namespace for maven.build.container.image.* for Docker images produced either by the Spring Boot Maven Plugin or by the Google Jib Maven Plugin and IMO it works well.

👍

on first glance, it may make sense to move container.registry.url and container.registry.username to maven.build.container.registry.*, which matches maven.build.container.image.*, and avoids some confusion with container.* resource attributes spec

@trask I followed your guidance and we now have maven.build.container.*and maven.build.repository.*. Please see updated PR description and updated README.md

anuraaga

So sorry I just merged in the change to error prone knowing that it is likely annoying for this PR. If you are able to merge main and fix issues that come up that would be great, otherwise I can do it to this PR tomorrow.

# Conflicts: # maven-extension/src/main/java/io/opentelemetry/maven/OtelExecutionListener.java # maven-extension/src/main/java/io/opentelemetry/maven/OtelUtils.java # maven-extension/src/main/java/io/opentelemetry/maven/semconv/MavenOtelSemanticAttributes.java

…the SPI mechanism

anuraaga

Thanks just small points

...sion/src/main/java/io/opentelemetry/maven/handler/MojoGoalExecutionHandlerConfiguration.java

anuraaga · 2021-12-21T00:29:16Z

...sion/src/main/java/io/opentelemetry/maven/handler/MojoGoalExecutionHandlerConfiguration.java

+                    .forEach(
+                        goal -> {
+                          MojoGoalExecutionHandler previousHandler =
+                              mojoGoalExecutionHandlers.put(goal, handler);


Think you can use toMap since it will throw on duplicate keys

https://docs.oracle.com/javase/8/docs/api/java/util/stream/Collectors.html#toMap-java.util.function.Function-java.util.function.Function-

Thanks @anuraaga . Could you please give me more guidance on this one?
I don't catch how I can use stream.collect(Collectors.toMap()) as I have a double loop 👼

Oops - yeah didn't think enough about it, stream doesn't seem good for this case

anuraaga · 2021-12-22T02:37:14Z

...sion/src/main/java/io/opentelemetry/maven/handler/MojoGoalExecutionHandlerConfiguration.java

+                    .forEach(
+                        goal -> {
+                          MojoGoalExecutionHandler previousHandler =
+                              mojoGoalExecutionHandlers.put(goal, handler);


Oops - yeah didn't think enough about it, stream doesn't seem good for this case

cyrille-leclerc · 2022-01-03T15:05:11Z

@ipleten I see on jenkinsci/opentelemetry-plugin#281 that you have CI pipelines build Java applications and packaging them as Docker images.

Could you please by any chance:

Indicate us how you build Docker images in your CI pipelines? A Maven / Gradle plugin? A shell script invoking docker build? ...
Have a look at the visualization of Maven + Docker builds we bring with this Pull Request and share feedback on this (screenshot above )?

# Conflicts: # maven-extension/src/main/java/io/opentelemetry/maven/OtelExecutionListener.java

cyrille-leclerc added 3 commits December 6, 2021 10:09

Capture details on mojo goal executions: deploy:deploy, spring-boot:b…

349af79

…uild-image

Merge remote-tracking branch 'origin/main' into capture-details-on-mo…

84b8a8f

…jo-goal-executions

Update screenshot

baab936

cyrille-leclerc changed the title ~~Capture details on mojo goal executions: deploy:deployand spring-boot:build-image~~ [maven-extension] Capture details on mojo goal executions: deploy:deployand spring-boot:build-image Dec 6, 2021

Make spotless happy

d2877d7

kenfinnigan reviewed Dec 6, 2021

View reviewed changes

maven-extension/src/main/java/io/opentelemetry/maven/handler/DeployDeployHandler.java Outdated Show resolved Hide resolved

maven-extension/src/main/java/io/opentelemetry/maven/OtelExecutionListener.java Outdated Show resolved Hide resolved

cyrille-leclerc added 2 commits December 7, 2021 21:34

Clarify TODO + better class name

9a78861

Improve code

a20efbf

kenfinnigan reviewed Dec 9, 2021

View reviewed changes

maven-extension/src/main/java/io/opentelemetry/maven/OtelExecutionListener.java Show resolved Hide resolved

kenfinnigan reviewed Dec 9, 2021

View reviewed changes

maven-extension/src/main/java/io/opentelemetry/maven/handler/MojoGoalExecutionHandler.java Outdated Show resolved Hide resolved

cyrille-leclerc added 2 commits December 10, 2021 14:55

Code cleanup

e7525a3

Code cleanup

36e71ab

cyrille-leclerc commented Dec 10, 2021

View reviewed changes

maven-extension/README.md Outdated Show resolved Hide resolved

Add handlers for snyk:test and snyk:monitor

a7dd06f

cyrille-leclerc added 2 commits December 14, 2021 16:52

Add support for the Google Maven Jib Plugin

a589e3f

Add unit tests, code cleanup

4fcbf1f

cyrille-leclerc changed the title ~~[maven-extension] Capture details on mojo goal executions: deploy:deployand spring-boot:build-image~~ [maven-extension] Capture details on mojo goal executions: deploy:deploy, spring-boot:build-image, jib:build, snyk:test, snyk:monitor Dec 14, 2021

cyrille-leclerc added 2 commits December 15, 2021 15:50

Better README.md

58e9897

Better README.md

2021e01

cyrille-leclerc marked this pull request as ready for review December 15, 2021 22:12

cyrille-leclerc requested a review from a team December 15, 2021 22:12

anuraaga reviewed Dec 16, 2021

View reviewed changes

cyrille-leclerc added 2 commits December 16, 2021 09:41

Cleanup code

23b761e

Iterate on the semantic conventions

117c21f

anuraaga reviewed Dec 16, 2021

View reviewed changes

cyrille-leclerc added 5 commits December 17, 2021 11:14

Replace Optional by `@Nullable

d815604

Make the built-in MojoGoalExecutionHandlers non-public while keeping …

d393bea

…the SPI mechanism

Make the built-in MojoGoalExecutionHandlers non-public while keeping …

d965f0f

…the SPI mechanism

spotless apply

34adb69

Stop capturing auth username waiting for config flag to disable this

a043666

anuraaga reviewed Dec 21, 2021

View reviewed changes

cyrille-leclerc added 2 commits December 21, 2021 09:30

Fix Javadocs

91d3630

Fix typo

8fc2d57

anuraaga approved these changes Dec 22, 2021

View reviewed changes

Merge branch 'main' into capture-details-on-mojo-goal-executions

81aaeb1

# Conflicts: # maven-extension/src/main/java/io/opentelemetry/maven/OtelExecutionListener.java

cyrille-leclerc force-pushed the capture-details-on-mojo-goal-executions branch from 3d82fd6 to 81aaeb1 Compare January 3, 2022 23:05

trask merged commit 2c02e42 into open-telemetry:main Jan 3, 2022

cyrille-leclerc added the component: maven-extension label Jan 12, 2022

cyrille-leclerc deleted the capture-details-on-mojo-goal-executions branch January 16, 2022 20:59

cyrille-leclerc added this to the v1.10.0 milestone Jan 19, 2022

v1v mentioned this pull request Feb 10, 2022

pom: bump opentelemetry-extension 1.10.0 elastic/apm-agent-java#2464

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[maven-extension] Capture details on mojo goal executions: `deploy:deploy`, `spring-boot:build-image`, `jib:build`, `snyk:test`, `snyk:monitor` #146

[maven-extension] Capture details on mojo goal executions: `deploy:deploy`, `spring-boot:build-image`, `jib:build`, `snyk:test`, `snyk:monitor` #146

cyrille-leclerc commented Dec 6, 2021 •

edited

Loading

cyrille-leclerc commented Dec 10, 2021

cyrille-leclerc commented Dec 15, 2021

trask commented Dec 15, 2021

anuraaga Dec 16, 2021

cyrille-leclerc Dec 16, 2021

anuraaga Dec 16, 2021

cyrille-leclerc Dec 17, 2021

anuraaga Dec 16, 2021

cyrille-leclerc Dec 16, 2021

cyrille-leclerc Dec 17, 2021

cyrille-leclerc commented Dec 16, 2021 •

edited

Loading

anuraaga left a comment

anuraaga left a comment

anuraaga Dec 21, 2021

cyrille-leclerc Dec 21, 2021

anuraaga Dec 22, 2021

anuraaga Dec 22, 2021

cyrille-leclerc commented Jan 3, 2022

		@@ -0,0 +1,5 @@
		io.opentelemetry.maven.handler.GoogleJibBuildHandler

[maven-extension] Capture details on mojo goal executions: deploy:deploy, spring-boot:build-image, jib:build, snyk:test, snyk:monitor #146

[maven-extension] Capture details on mojo goal executions: deploy:deploy, spring-boot:build-image, jib:build, snyk:test, snyk:monitor #146

Conversation

cyrille-leclerc commented Dec 6, 2021 • edited Loading

cyrille-leclerc commented Dec 10, 2021

cyrille-leclerc commented Dec 15, 2021

trask commented Dec 15, 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

cyrille-leclerc commented Dec 16, 2021 • edited Loading

anuraaga left a comment

Choose a reason for hiding this comment

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

cyrille-leclerc commented Jan 3, 2022

[maven-extension] Capture details on mojo goal executions: `deploy:deploy`, `spring-boot:build-image`, `jib:build`, `snyk:test`, `snyk:monitor` #146

[maven-extension] Capture details on mojo goal executions: `deploy:deploy`, `spring-boot:build-image`, `jib:build`, `snyk:test`, `snyk:monitor` #146

cyrille-leclerc commented Dec 6, 2021 •

edited

Loading

cyrille-leclerc commented Dec 16, 2021 •

edited

Loading