Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Verify OpenCensus bridge is compatible with the specificaiton #4514

Closed
MrAlias opened this issue Sep 14, 2023 · 5 comments
Closed

Verify OpenCensus bridge is compatible with the specificaiton #4514

MrAlias opened this issue Sep 14, 2023 · 5 comments
Assignees
Labels
pkg:bridges Related to a bridge package
Milestone

Comments

@MrAlias
Copy link
Contributor

MrAlias commented Sep 14, 2023

Specification: https://github.com/open-telemetry/opentelemetry-specification/blob/v1.25.0/specification/compatibility/opencensus.md

Create an issue for anything not compliant.

@MrAlias MrAlias added the pkg:bridges Related to a bridge package label Sep 14, 2023
@dashpole
Copy link
Contributor

dashpole commented Sep 29, 2023

Trace Bridge

The trace bridge is provided as a shim layer implementing the OpenCensus Trace API using the OpenTelemetry Trace API. This layer MUST NOT rely on implementation specific details of the OpenTelemetry SDK.

It does not depend on the trace SDK:

require (
github.com/stretchr/testify v1.8.4
go.opencensus.io v0.24.0
go.opentelemetry.io/otel v1.19.0
go.opentelemetry.io/otel/sdk v1.19.0
go.opentelemetry.io/otel/sdk/metric v1.19.0
go.opentelemetry.io/otel/trace v1.19.0
)

This Shim Layer MUST NOT publicly expose any upstream OpenTelemetry API.

The bridge will accept a TracerProvider, but does not expose any OpenTelemetry API interfaces in its public API.

Easy for users to use, ideally no change to their code

The InstallTraceBridge() function (added in #4567) is as simple as we could possibly get.

Maintain parent-child span relationship between applications and libraries

The bridge does this, as tested here: https://github.com/open-telemetry/opentelemetry-go/blob/main/bridge/opencensus/test/bridge_test.go#L32

This component MUST be an optional dependency.

The bridge is in its own module, and thus is fully optional.

When the shim is in place, all OpenCensus Spans MUST be sent through an OpenTelemetry Tracer as specified for the OpenTelemetry API.

The current API, NewTracer, accepts an OpenTelemetry Tracer, which it uses to record spans from OpenCensus:

func NewTracer(tracer trace.Tracer) octrace.Tracer {

After #4567, this will still be the case in InstallTraceBridge: https://github.com/open-telemetry/opentelemetry-go/pull/4567/files#diff-ee40e63756bf77fde5a66bcf047e3bee7ae65207d633d2a4dc9139a8b7d6362fR40

This mechanism SHOULD be seamless to the user in languages that allow discovery and auto injection of dependencies.

I don't believe this applies to Go.

All specified methods in OpenCensus will delegate to the underlying Span of OpenTelemetry.

The bridge span wrapper delegates all OpenCensus calls to the OpenTelemetry tracer:

// Span is an OpenCensus SpanInterface wrapper for an OpenTelemetry Span.
type Span struct {
otelSpan trace.Span

The OpenCensus Trace Bridge is compliant with the specification.

@dashpole dashpole self-assigned this Sep 29, 2023
@dashpole
Copy link
Contributor

Metrics Bridge

OpenTelemetry will provide an OpenCensus-Metrics-Shim component which implements the OpenTelemetry MetricProducer interface.

opencensus.NewMetricProducer returns a metric.Producer: https://github.com/open-telemetry/opentelemetry-go/blob/main/bridge/opencensus/metric.go#L37

When Produce() is invoked, the shim collects metrics from the OpenCensus global state, converts the metrics to an OpenTelemetry metrics batch, and returns.

producers := p.manager.GetAll()
data := []*ocmetricdata.Metric{}
for _, ocProducer := range producers {
data = append(data, ocProducer.Read()...)
}
otelmetrics, err := internal.ConvertMetrics(data)
if len(otelmetrics) == 0 {
return nil, err
}
return []metricdata.ScopeMetrics{{
Scope: instrumentation.Scope{
Name: scopeName,
},
Metrics: otelmetrics,
}}, err

This component MUST be an optional dependency

The bridge is in its own go module, which makes it optional for users to use.

MUST NOT require OpenTelemetry to be included in OpenCensus API distributions. SHOULD NOT require OpenCensus to depend on OpenTelemetry at runtime

OpenCensus does not depend on OpenTelemetry

MUST require few or no changes to OpenCensus

No changes were required to OpenCensus for this bridge.

MUST be compatible with push and pull exporters

ManualReader, PeriodicReader, and the prometheus exporter all accept metric.Producer as a source of external metrics.

MUST support Gauges, Counters, Cumulative Histograms, and Summaries

We do not support summaries today. #4571

Gauges, Counters, Cumulative Histograms:

case ocmetricdata.TypeGaugeInt64:
return convertGauge[int64](labelKeys, metric.TimeSeries)
case ocmetricdata.TypeGaugeFloat64:
return convertGauge[float64](labelKeys, metric.TimeSeries)
case ocmetricdata.TypeCumulativeInt64:
return convertSum[int64](labelKeys, metric.TimeSeries)
case ocmetricdata.TypeCumulativeFloat64:
return convertSum[float64](labelKeys, metric.TimeSeries)
case ocmetricdata.TypeCumulativeDistribution:
return convertHistogram(labelKeys, metric.TimeSeries)

MUST support exemplar span context in language that provide utilities for recording span context in exemplars

We do not support exemplars today: #4572

The shim MUST discard the resource attached to OpenCensus metrics, and insert the resource provided during initialization, or fall back to the the default OpenTelemetry resource.

We discard the resource attached to OpenCensus, since the bridge does not return a resource. The OTel metrics SDK inserts the resource provided during initialization, or falls back to the default.

The shim MUST add an instrumentation scope name and version which identifies the shim.

We do add a scope with a name, but do not add a version: #4573

The shim can be passed as an option to an OpenTelemetry MetricReader when configuring the OpenTelemetry SDK. This enables the bridge to work with both push and pull metric exporters.

The OpenCensus bridge can be passed to PeriodicReader, ManualReader, and the Prometheus exporter.

@dashpole
Copy link
Contributor

@dashpole
Copy link
Contributor

dashpole commented Nov 2, 2023

Based on the above, I think the bridge is compatible with the specification. If someone else can review my analysis above, I think we can close.

@pellared
Copy link
Member

pellared commented Nov 21, 2023

To my understanding we are compliant with the specification.

However, I created #4729.

@XSAM XSAM added this to the untracked milestone Nov 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg:bridges Related to a bridge package
Projects
No open projects
Development

No branches or pull requests

4 participants