[DO NOT MERGE] PoC: Sharing code across multiple Go modules

[pellared]

Towards #3548

This PR is a proposal on how we can deal with sharing common code across multiple Go modules.

[codecov]

Codecov Report

Merging #3841 (93dc9e5) into main (3015c86) will decrease coverage by 0.3%.
The diff coverage is 55.1%.

❗ Current head 93dc9e5 differs from pull request most recent head 435be78. Consider uploading reports for the commit 435be78 to get more accurate results

Additional details and impacted files

@@           Coverage Diff           @@
##            main   #3841     +/-   ##
=======================================
- Coverage   81.7%   81.4%   -0.3%     
=======================================
  Files        170     172      +2     
  Lines      12474   12571     +97     
=======================================
+ Hits       10192   10237     +45     
- Misses      2067    2113     +46     
- Partials     215     221      +6     

Impacted Files	Coverage Δ
exporters/jaeger/env_shared.go	21.8% <21.8%> (ø)
internal/shared/gocpy/gocpy.go	39.4% <39.4%> (ø)
exporters/jaeger/uploader.go	47.5% <100.0%> (ø)
internal/shared/env.go	100.0% <100.0%> (ø)
sdk/trace/batch_span_processor.go	81.1% <0.0%> (-0.9%)	⬇️
exporters/jaeger/jaeger.go	90.3% <0.0%> (-0.9%)	⬇️
bridge/opentracing/migration/defer.go	100.0% <0.0%> (ø)

[Aneurysm9]

I'd think we should do this from the beginning. The collector may find itself in this situation as well.

[pellared]

If everyone agrees and accepts the current proposal then I can go for it.

CC @open-telemetry/build-tools-approvers @open-telemetry/collector-approvers

[MrAlias]

How do we plan to address the internal otel logging functionality?

[pellared]

I will take a look tomorrow. Can you send a hyperlink with an example code that you are mentioning?

[MrAlias]

opentelemetry-go/internal/global/internal_logging.go

Line 61 in 3df561e

func Debug(msg string, keysAndValues ...interface{}) {

[pellared]

TL;DR;
I think that it would make sense to extract the internal logging to a public Go module with package name e.g. loginternal.

Using code generation (gocpy)
Initially I was thinking about adding func Logger() to the go.opentelemetry.io/otel package and making the Debug Info and Error as shared. But then I realized that this would introduce a cyclic dependency, becasue logging is used internally e.g. in internal/global/state.go....

Creating new module/package
Then I realized that probably exposing loginternal may be good as it would also allow others (e.g. instrumentation library authors) to use the same internal logging infrastructure. Right now the instrumentation library authors would have to create their own logging abstractions as getting the globalLogger is not possible using the OTel Go functions. Take notice that OTel Go users have access to their own logger if they set it. Therefore, I think it would be useful to make the internal logging functions publicly available.

(Possibly) Motivating example. In Splunk distribution of OTel Go we set a different default logger: https://github.com/signalfx/splunk-otel-go/blob/8cef1ed33297c9e8f348346b6b0b2da0225ab89e/distro/otel.go#LL86

And then if we want to use the same logger we need to pass it everywhere (or we could introduce our own global):

• https://github.com/signalfx/splunk-otel-go/blob/8cef1ed33297c9e8f348346b6b0b2da0225ab89e/distro/otel.go#L84
• https://github.com/signalfx/splunk-otel-go/blob/8cef1ed33297c9e8f348346b6b0b2da0225ab89e/distro/otel.go#L90

I think that it would be more maintainable and consistent if we could use the same Debug, Info, Error functions as OTel Go uses internally.

I find the internal logging as good use case for @Aneurysm9 alternatice proposal (source) on how we can share code between modules:

Alternately, we can extract this into a separate module with a stable public API. It might limit our flexibility going forward, but should be the least surprising in all use cases.

Of course, this approach should be used in moderation as it increases the exported API surface.

PS. I hope that my comment is "understandable". Feel free to ask question and let me know what is not clear.

PS2. Great question.

[Aneurysm9]

What about using text/template and making the input files templates? That would allow replacing the package name and canonical import comment and eliminate the prefix check on each line.

[pellared]

I had the same idea initially 😉

However, I found it easier to use actual code than writing Go source code templates. We get IDE, linting support when writing the shared code what we could not say about templates. We only need to replace the package name which is not anything complex.

eliminate the prefix check on each line.

If the performance is the concern, this could be easily improved by making sure that after the package name is replaced then we are not longer checking the line prefix. So the prefix check would be done only for the first couple of lines.

[pellared]

If the performance is the concern, this could be easily improved by making sure that after the package name is replaced then we are not longer checking the line prefix. So the prefix check would be done only for the first couple of lines.

I did it in open-telemetry/opentelemetry-go-build-tools#275