open-telemetry / opentelemetry-dotnet

The OpenTelemetry .NET Client
https://opentelemetry.io
Apache License 2.0
3.17k stars 751 forks source link

Rename packages #3469

Open alanwest opened 2 years ago

alanwest commented 2 years ago

Packages to rename

The following packages in both the main and the contrib repos should be renamed to better describe what they contain.

Current name Suggested name Notes
OpenTelemetry.Extensions.Serilog OpenTelemetry.Appenders.Serilog 1, 6
OpenTelemetry.Extensions.EventSource OpenTelemetry.Appenders.EventSource 1, 6
OpenTelemetry.Instrumentation.EventCounters OpenTelemetry.Shims.EventCounters 1
OpenTelemetry.Extensions.Docker OpenTelemetry.ResourceDetectors.Container 2
OpenTelemetry.Extensions.Propagators OpenTelemetry.Propagators 2, 5
OpenTelemetry.Extensions.PersistentStorage.Abstractions OpenTelemetry.PersistentStorage.Abstractions 2
OpenTelemetry.Extensions.PersistentStorage OpenTelemetry.PersistentStorage.FileSystem 2
OpenTelemetry.Contrib.Instrumentation.AWS OpenTelemetry.Instrumentation.AWS 3
OpenTelemetry.Contrib.Extensions.AWSXRay OpenTelemetry.??.AWSXRay 3
OpenTelemetry.Extensions.AzureMonitor OpenTelemetry.Samplers.AzureMonitor 3
OpenTelemetry.Extensions ? 4

[1]: A shim is a package that integrates a library with OpenTelemetry - e.g. "I use library xyz to produce [traces | metrics | logs], I need to use a shim package to integrate it with OpenTelemetry".

[2]: The name of an SDK extension package should contain the type of extension it contains - e.g., Exporter, ResourceDetectors, Propagators, etc. We prefer pluralized names, but may consider exceptions to this rule. For example, PersistentStorages sounds wrong. The inspiration for preferring pluralized names comes from the .NET design guidelines for namespace naming.

[3]: The naming of vendor-specific packages should be up to the vendor. However, we should make a suggestion and provide vendors with guidance.

[4]: OpenTelemetry.Extensions is a bit of an odd-ball. It contains a processor for both traces and logs, so should it be named OpenTelemetry.Processors.Something?

[5]: Not sure we can rename OpenTelemetry.Extensions.Propagators since it already has a stable release.

[6]: The initial development of the logging shims is on the main-logs branch. The will probably move to the contrib repo.

Packages to leave alone

(expand me) For reference, this is a list of the remaining packages. If there are packages in this list you think should be renamed, please comment. | Package name | | ------------ | | OpenTelemetry | | OpenTelemetry.Api | | OpenTelemetry.Exporter.Console | | OpenTelemetry.Exporter.Geneva | | OpenTelemetry.Exporter.InMemory | | OpenTelemetry.Exporter.Instana | | OpenTelemetry.Exporter.Jaeger | | OpenTelemetry.Exporter.OpenTelemetryProtocol | | OpenTelemetry.Exporter.Prometheus.AspNetCore | | OpenTelemetry.Exporter.Prometheus.HttpListener | | OpenTelemetry.Exporter.Stackdriver | | OpenTelemetry.Exporter.Zipkin | | OpenTelemetry.Exporter.ZPages | | OpenTelemetry.Extensions.DependencyInjection | | OpenTelemetry.Extensions.Hosting | | OpenTelemetry.Instrumentation.AspNet | | OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule | | OpenTelemetry.Instrumentation.AspNetCore | | OpenTelemetry.Instrumentation.AWSLambda | | OpenTelemetry.Instrumentation.ElasticsearchClient | | OpenTelemetry.Instrumentation.EntityFrameworkCore | | OpenTelemetry.Instrumentation.GrpcCore | | OpenTelemetry.Instrumentation.GrpcNetClient | | OpenTelemetry.Instrumentation.Hangfire | | OpenTelemetry.Instrumentation.Http | | OpenTelemetry.Instrumentation.MassTransit | | OpenTelemetry.Instrumentation.MySqlData | | OpenTelemetry.Instrumentation.Owin | | OpenTelemetry.Instrumentation.Process | | OpenTelemetry.Instrumentation.Quartz | | OpenTelemetry.Instrumentation.Runtime | | OpenTelemetry.Instrumentation.SqlClient | | OpenTelemetry.Instrumentation.StackExchangeRedis | | OpenTelemetry.Instrumentation.Wcf | | OpenTelemetry.SemanticConventions | | OpenTelemetry.Shims.OpenTracing |
reyang commented 2 years ago

Proposal Shim better describes what the package does for you. I think Shim also describes what the OpenTelemetry.Instrumentation.EventCounters package does. So for example:

OpenTelemetry.Extensions.Serilog becomes OpenTelemetry.Shims.Serilog

+1

reecebradley commented 1 year ago

[1]: A shim is a package that integrates a library with OpenTelemetry - e.g. "I use library xyz to produce [traces | metrics | logs], I need to use a shim package to integrate it with OpenTelemetry".

I agree.

[2]: The name of an SDK extension package should contain the type of extension it contains - e.g., Exporter, ResourceDetector, Propagator, etc. Should they be pluralize or not, i.e., Exporter vs. Exporters? [3]: The naming of vendor-specific packages should be up to the vendor. However, we should make a suggestion and provide vendors with guidance.

I agrees :-)

If we have a good set of extension points that are commonly done, those names, like Exporters or Shims would be good to document. I always how this made all the packages start to feel cohesive or like, one person authored it. Makes it easier for consumers to navigate all the packages after learning the domain.

[5]: Not sure we can rename OpenTelemetry.Extensions.Propagators since it already has a stable release.

:-(

CodeBlanch commented 1 year ago

@alanwest I could live with "Shim" but I think "Adapter" could also work. In the case of Serilog & EventSource, maybe even "Appender" since the spec has run with that term? 🤷‍♀️

@vishweshbankwar How would you feel about renaming OpenTelemetry.PersistentStorage -> OpenTelemetry.PersistentStorage.FileSystem? Imagine side-by-side with some other implementations...

OpenTelemetry.PersistentStorage.Abstractions OpenTelemetry.PersistentStorage.FileSystem OpenTelemetry.PersistentStorage.MessageQueue OpenTelemetry.PersistentStorage.Database

Not saying we will ever ship those, but I think the convention leads itself to a happy place 😄

alanwest commented 1 year ago

I could live with "Shim" but I think "Adapter" could also work.

I like Adapter or even Bridge. Wasn't a fan of Shims anyways, but chose it because of our existing OpenTelemetry.Shims.OpenTracing package. Though, we could rename that too since we've never shipped a stable version of it.

At least for OpenTracing stuff:

Adapter feels familiar in .NET because of DataAdapter

reyang commented 1 year ago

I like Adapter or even Bridge.

"Bridge" is normally used to refer to a bi-directional thing.

Wasn't a fan of Shims anyways, but chose it because of our existing OpenTelemetry.Shims.OpenTracing package. Though, we could rename that too since we've never shipped a stable version of it.

FYI - The spec is using the word "shim" https://github.com/open-telemetry/opentelemetry-specification/blob/094fe9f34bc2fdb9797c4437c6062dcf61760de0/specification/compatibility/opentracing.md.

alanwest commented 1 year ago

The spec is using the word "shim"

And apparently matches wikipedia's definition quite well https://en.wikipedia.org/wiki/Shim_(computing)

reyang commented 1 year ago

The spec is using the word "shim"

And apparently matches wikipedia's definition quite well https://en.wikipedia.org/wiki/Shim_(computing)

IIRC it started as "bridge" then there was a holy war in the spec SIG and folks aligned on the term "shim"...

vishweshbankwar commented 1 year ago

@alanwest I could live with "Shim" but I think "Adapter" could also work. In the case of Serilog & EventSource, maybe even "Appender" since the spec has run with that term? 🤷‍♀️

@vishweshbankwar How would you feel about renaming OpenTelemetry.PersistentStorage -> OpenTelemetry.PersistentStorage.FileSystem? Imagine side-by-side with some other implementations...

OpenTelemetry.PersistentStorage.Abstractions OpenTelemetry.PersistentStorage.FileSystem OpenTelemetry.PersistentStorage.MessageQueue OpenTelemetry.PersistentStorage.Database

Not saying we will ever ship those, but I think the convention leads itself to a happy place 😄

I like it :). It is more clear and leaves room for other implementations to use the prefix.

alanwest commented 1 year ago

I've updated the PR description with a few of the changes discussed.

1) OpenTelemetry.PersistentStorage -> OpenTelemetry.PersistentStorage.FileSystem 2) OpenTelemetry.Shims.Serilog -> OpenTelemetry.Appenders.Serilog 3) OpenTelemetry.Shims.EventSource -> OpenTelemetry.Appenders.EventSource

Instead of the word using the word Shim for the last two these, I liked @CodeBlanch's suggestion of using Appender since this this a term used by the spec. I'd still be ok with Shim, but interested in other folks' thoughts.

alanwest commented 1 year ago

With https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/1123, we have asserted a preference for pluralizing words in our package names.

I've updated note 2 in this issue regarding pluralization. Let me know if this sounds reasonable to folks.

[2]: The name of an SDK extension package should contain the type of extension it contains - e.g., Exporter, ResourceDetectors, Propagators, etc. We prefer pluralized names, but may consider exceptions to this rule. For example, PersistentStorages sounds wrong. The inspiration for preferring pluralized names comes from the .NET design guidelines for namespace naming.

cijothomas commented 1 year ago

OpenTelemetry.Appenders.Serilog --> OpenTelemetry.Appender.Serilog

Wondering if Appenders was a conscious name for some reason?

CodeBlanch commented 5 months ago

[Cross-linking]

Over on https://github.com/open-telemetry/opentelemetry-dotnet-contrib/issues/1610 the suggestion is OpenTelemetry.ResourceDetectors.[ComponentName] -> OpenTelemetry.Resource.[ComponentName] or OpenTelemetry.Resources.[ComponentName]