Rename two more modules / maven artifactIds?

[trask]

The opentelemetry-api-beta artifactId is different from other instrumentation, which all start with opentelemetry-auto-*, and the opentelemetry-auto-annotations artifactId is different, because it has no version:

opentelemetry-api-beta
opentelemetry-auto-akka-http-10.0
opentelemetry-auto-annotations
opentelemetry-auto-apache-httpasyncclient-4.0
opentelemetry-auto-apache-httpclient-2.0
opentelemetry-auto-apache-httpclient-4.0
opentelemetry-auto-aws-sdk-1.11
opentelemetry-auto-aws-sdk-2.2
opentelemetry-auto-cassandra-3.0
...

What do you think about renaming the opentelemetry-api-beta module to otel-api-beta, and applying the normal artifactId prefix, which would make the artifactId opentelemetry-auto-otel-api-beta?

Or keeping the same module name, but still applying the normal artifact prefix, which would make the artifactId opentelemetry-auto-opentelemetry-api-beta?

For opentelemetry-auto-annotations, it serves two purposes, one is for otel annotations, and one is for other annotations. What do you think of versioning it based on the otel annotation version that it supports? So for now, making it annotations-beta (similar to otel-api-beta) and then moving to real version numbers once we hit 1.0.

Or, another option is to split up the annotations module:

• :instrumentation:annotations:annotations-otel-beta (beta to be replaced by 1.0 in GA)
• :instrumentation:annotations:annotations-other (with no version, as it targets several different annotations from different libraries)
• :instrumentation:annotations:annotations-common (we have some configuration parsing code that is shared)

[iNikem]

How about moving support for WithSpan annotation into opentelemetry-api-beta?

[anuraaga]

The naming does seem confusing, especially once it's out of beta, our artifact name opentelemetry-api-1.0 would look so close to that in a highly related project, making build files harder to read. Adding the normal opentelemetry-auto- prefix seems like a must, but one thought is, is it practical to use the agent without the module, which is handling the shading for us? We could merge it into agent-tooling if it's a critical part of the agent and the naming problem would go away I guess. FWIW, I was really surprised to find next to library tooling like okhttp this module that was messing up my IntelliJ debugging experience ;)

Merging WithSpan in seems fine too to me, there is an argument that since it's an extension it should stay somewhat separate, but it seems ok to me.

[trask]

is it practical to use the agent without the module [:instrumentation:opentelemetry-api-beta]

At this point at least, :instrumentation:opentelemetry-api-beta is optional, and only applied if application is using manual instrumentation (either our manual instrumentation or straight OpenTelemetry API).

messing up my IntelliJ debugging experience

Can you open an issue to track this? I don't think I've run into this.

How about moving support for WithSpan annotation into opentelemetry-api-beta?

I'm mixed on this.

Both WithSpan and other annotations share the config property traceMethodsExclude, so that makes me think they should be aligned.

And they both share MethodsConfigurationParser, although we could push that up into Config.

On the other hand WithSpan is part of opentelemetry, so at a high level, instrumentation of WithSpan does seem better aligned with instrumentation of OpenTelemetry API.

Ok I'm not mixed on this anymore, I agree with re-alignment 😄.

What do you think of

• :instrumentation:otel:otel-api-beta
• :instrumentation:otel:otel-annotations-beta

I'd also be happy with just combining WithSpan instrumentation into :instrumentation:otel:otel-api-beta, and we can always break it out later if we want.

[iNikem]

It is always nice to read Trask’s train of thoughts :)

I don’t think separate module just for 1 annotation is justified.

[anuraaga]

only applied if application is using manual instrumentation

Ah I guess I got it because I use the API just to get the current span ID for some debugging. Dunno how representative of usage this is, but I'd be surprised if long term we can get away with users not using the API at all - one of the first things I'd do in the app is get the current span and stuff tags in for whether client == ANDROID vs IOS for a mobile API server for example. So I guess in practice, we should still reasonable usage of it.

Though that's a lot of context (not that one, that's coming up :P), but in more practical terms, the better question would have been do we expect any repackaged agent to function properly without this? I think the potential for manual API usage will exist for all agents and they need this, whereas they may decide to exclude some of the library instrumentations. So if it seems required to create any sort of OTel agent anyways, there doesn't seem to be that much value in having a separate artifact.

I also notice io.grpc.Context is instrumented, is my reading right that it's likely most grpc apps will be instrumented by the module?

Can you open an issue to track this? I don't think I've run into this.

Messed was the wrong word I think, more just confusing, was seeing packages I didn't expect in the debugger. Don't think this can be avoided when doing shading

I don’t think separate module just for 1 annotation is justified.

I think it's a lot of logic, just to process 1 measly annotation ;) But personally agree that combining and possibly splitting out later is fine, there's less worry about refactoring agent modules than there is if this was manual instrumentation.

[trask]

I also notice io.grpc.Context is instrumented

Yeah, this instrumentation needs to be reviewed, I did not have a good grasp of context propagation when I wrote it.

is my reading right that it's likely most grpc apps will be instrumented by the module?

Only if OpenTelemetry API is also on the classpath, due to https://github.com/open-telemetry/opentelemetry-java-instrumentation/blob/master/instrumentation/opentelemetry-api-beta/src/main/java/io/opentelemetry/auto/instrumentation/opentelemetryapi/GrpcContextInstrumentation.java#L64-L65

do we expect any repackaged agent to function properly without :instrumentation:opentelemetry-api-beta?

I think the same could be said for :instrumentation:java-classloader and :instrumentation:java-concurrent.

There's benefit to it still being a separate module (e.g. muzzle). Or are you suggesting keeping it as a separate module, but not publish it as a separate artifact?

[anuraaga]

There's benefit to it still being a separate module (e.g. muzzle).

I didn't know about the benefits :P But yeah the suggestion was mainly about the published artifacts, and indeed it probably applies to other modules too.

[trask]

Ok, based on discussion, a concrete proposal:

• Rename the module name to :instrumentation:otel-api-beta (I think it still helps to differentiate the module name even if we don't publish the artifact)
• Keep :instrumentation:annotations module name as is
• Move WithSpan instrumentation from :instrumentation:annotations to :instrumentation:otel-api-beta

I don't have much opinion about whether to publish these "critical instrumentation" artifacts (otel-api-beta, java-classloader, java-concurrent) separately or bundle them into agent-tooling.

One possible advantage to publishing them separately is in case a vendor wants to override/fork/patch one of them, but I'm thinking that's probably not a real use case.

[anuraaga]

LGTM - took a second look and just realized we support all these different annotations that's pretty cool. Might rename it to external-annotations, vendor-annotations or similar to make that more obvious with the move of WithSpan.

[trask]

Rename the module name to :instrumentation:otel-api-beta (I think it still helps to differentiate the module name even if we don't publish the artifact)

Module name is currently :instrumentation:opentelemetry-auto-opentelemetry-api-beta which seems good to me, I don't think I have preference between that and abbreviating the second opentelemetry to otel.

Move WithSpan instrumentation from :instrumentation:annotations to :instrumentation:otel-api-beta

Done in #929