signalfx / gdi-specification

Splunk GDI specification for cross-repository compatibility
Apache License 2.0
3 stars 15 forks source link

GA splunk-otel-dotnet #248

Closed Kielek closed 1 year ago

Kielek commented 1 year ago

Which GDI repository do you wish to GA?

Name and link to repository: Splunk Distribution of OpenTelemetry .NET - https://github.com/signalfx/splunk-otel-dotnet

Does the repository follow the latest tagged minor release in GDI specification?

How long has the GDI repository been public?

From the very beginning - first commit: https://github.com/signalfx/splunk-otel-dotnet/commit/805704f44c84228a881c24b1447a953564d5461e

Is the repository known to be used today?

yes - in the RC phase. It is also documented in O11y documentation.

Is there a date by which this approval is needed?**

August 21, 2023

Additional context

Make a official 1.0.0 release of this package.

mateuszrzeszutek commented 1 year ago

The repo does not grant write access to the signalfx/gdi-specification-approvers; but then again, no other repo does that (well, I may have not checked all of them). Since it clearly hasn't been needed before, I propose to remove this requirement instead.

Repo also grants write permissions to a bot account, but that's allowed on the main branch, so I think we should probably make a new GDI spec release with all the fixes soon and simply bump the version here to 1.6.0.

mateuszrzeszutek commented 1 year ago

Follows the configuration requirements, if appropriate.

SPLUNK_ACCESS_TOKENSPLUNK_REALMSPLUNK_TRACE_RESPONSE_HEADER_ENABLEDSPLUNK_PROFILER_* N/A

OTEL_SERVICE_NAME and the missing service.name warning ✅ OTEL_PROPAGATORS ✅ Span Collection Limits ✅ does not set unlimited on several properties, but they all fall back to OTEL_ATTRIBUTE_COUNT_LIMIT so effectively it's the same thing OTEL_TRACES_EXPORTER looks like the upstream's using http/protobuf

mateuszrzeszutek commented 1 year ago

Follows the semantic convention requirements, if appropriate.

telemetry.sdk.*telemetry.auto.versionprocess.* and process.runtime.* do not seem to be set anywhere; they're optional so that's fine splunk.distro.version

mateuszrzeszutek commented 1 year ago

.github/CODEOWNERS

CODEOWNERS does not specify @signalfx/gdi-specification-approvers @signalfx/gdi-specification-maintainers, but I believe that this is connected to the branch permissions thingy that I mentioned before, and probably actually unneeded. I'll submit a spec fix for that too.

mateuszrzeszutek commented 1 year ago

Releases

MUST update all examples in the Splunk OpenTelemetry example repository that depends on the repository to use the latest release.

❌ not done https://github.com/signalfx/tracing-examples/blob/f4af470b059b176dbb6767c6f9fb3aee232a42b7/opentelemetry-tracing/opentelemetry-dotnet/automatic/aspnetcore-and-mongodb/InstrumentContainer/Dockerfile#L5

done in https://github.com/signalfx/tracing-examples/pull/160/files

mateuszrzeszutek commented 1 year ago

Type specific requirements Instrumentation Library

Documents all supported configuration parameters. -- Splunk docs ✅ Documents how to configure manual instrumentation. -- docs ✅

Documents how to configure log correlation. ❌ missing, and it looks like the sfx version had that: https://docs.splunk.com/Observability/gdi/get-data-in/application/dotnet/instrumentation/connect-traces-logs.html

Done in https://github.com/splunk/public-o11y-docs/pull/933

Documents minimum supported version of each auto-instrumentation framework. ❌ missing I think? I could not find that in the upstream docs either. done in https://docs.splunk.com/Observability/gdi/get-data-in/application/otel-dotnet/dotnet-requirements.html

Although, I don't think that the minimum supported version of each auto-instrumentation framework docs requirement is well-formed, really; for instance, for Java we only document that in the upstream repo, there's no Splunk documentation on that.

mateuszrzeszutek commented 1 year ago

CODEOWNERS and permissions fixed in https://github.com/signalfx/splunk-otel-dotnet/pull/326

mateuszrzeszutek commented 1 year ago

Looks like all items have been resolved; I think we're ready to go 🚀