Skip to content

Commit

Permalink
revent end-users from implementing some interfaces (#1575)
Browse files Browse the repository at this point in the history
"otel/exporters/otlp/otlphttp".Option
"otel/exporters/stdout".Option
"otel/oteltest".Option
"otel/trace".TracerOption
"otel/trace".SpanOption
"otel/trace".EventOption
"otel/trace".LifeCycleOption
"otel/trace".InstrumentationOption
"otel/sdk/resource".Option
"otel/sdk/trace".ParentBasedSamplerOption
"otel/sdk/trace".ReadOnlySpan
"otel/sdk/trace".ReadWriteSpan

Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>
  • Loading branch information
punya and MrAlias authored Feb 24, 2021
1 parent 85e696d commit 37688ef
Show file tree
Hide file tree
Showing 9 changed files with 131 additions and 9 deletions.
15 changes: 15 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,21 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
- Renamed the `otel/label` package to `otel/attribute`. (#1541)
- Vendor the Jaeger exporter's dependency on Apache Thrift. (#1551)
- Stagger timestamps in exact aggregator tests. (#1569)
- Prevent end-users from implementing some interfaces (#1575)
```
"otel/exporters/otlp/otlphttp".Option
"otel/exporters/stdout".Option
"otel/oteltest".Option
"otel/trace".TracerOption
"otel/trace".SpanOption
"otel/trace".EventOption
"otel/trace".LifeCycleOption
"otel/trace".InstrumentationOption
"otel/sdk/resource".Option
"otel/sdk/trace".ParentBasedSamplerOption
"otel/sdk/trace".ReadOnlySpan
"otel/sdk/trace".ReadWriteSpan
```

### Removed

Expand Down
23 changes: 23 additions & 0 deletions exporters/otlp/otlphttp/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,11 @@ type config struct {
// Option applies an option to the HTTP driver.
type Option interface {
Apply(*config)

// A private method to prevent users implementing the
// interface and so future additions to it will not
// violate compatibility.
private()
}

type endpointOption string
Expand All @@ -71,6 +76,8 @@ func (o endpointOption) Apply(cfg *config) {
cfg.endpoint = (string)(o)
}

func (endpointOption) private() {}

// WithEndpoint allows one to set the address of the collector
// endpoint that the driver will use to send metrics and spans. If
// unset, it will instead try to use
Expand All @@ -86,6 +93,8 @@ func (o compressionOption) Apply(cfg *config) {
cfg.compression = (Compression)(o)
}

func (compressionOption) private() {}

// WithCompression tells the driver to compress the sent data.
func WithCompression(compression Compression) Option {
return (compressionOption)(compression)
Expand All @@ -97,6 +106,8 @@ func (o tracesURLPathOption) Apply(cfg *config) {
cfg.tracesURLPath = (string)(o)
}

func (tracesURLPathOption) private() {}

// WithTracesURLPath allows one to override the default URL path used
// for sending traces. If unset, DefaultTracesPath will be used.
func WithTracesURLPath(urlPath string) Option {
Expand All @@ -109,6 +120,8 @@ func (o metricsURLPathOption) Apply(cfg *config) {
cfg.metricsURLPath = (string)(o)
}

func (metricsURLPathOption) private() {}

// WithMetricsURLPath allows one to override the default URL path used
// for sending metrics. If unset, DefaultMetricsPath will be used.
func WithMetricsURLPath(urlPath string) Option {
Expand All @@ -121,6 +134,8 @@ func (o maxAttemptsOption) Apply(cfg *config) {
cfg.maxAttempts = (int)(o)
}

func (maxAttemptsOption) private() {}

// WithMaxAttempts allows one to override how many times the driver
// will try to send the payload in case of retryable errors. If unset,
// DefaultMaxAttempts will be used.
Expand All @@ -134,6 +149,8 @@ func (o backoffOption) Apply(cfg *config) {
cfg.backoff = (time.Duration)(o)
}

func (backoffOption) private() {}

// WithBackoff tells the driver to use the duration as a base of the
// exponential backoff strategy. If unset, DefaultBackoff will be
// used.
Expand All @@ -147,6 +164,8 @@ func (o *tlsClientConfigOption) Apply(cfg *config) {
cfg.tlsCfg = (*tls.Config)(o)
}

func (*tlsClientConfigOption) private() {}

// WithTLSClientConfig can be used to set up a custom TLS
// configuration for the client used to send payloads to the
// collector. Use it if you want to use a custom certificate.
Expand All @@ -160,6 +179,8 @@ func (insecureOption) Apply(cfg *config) {
cfg.insecure = true
}

func (insecureOption) private() {}

// WithInsecure tells the driver to connect to the collector using the
// HTTP scheme, instead of HTTPS.
func WithInsecure() Option {
Expand All @@ -172,6 +193,8 @@ func (o headersOption) Apply(cfg *config) {
cfg.headers = (map[string]string)(o)
}

func (headersOption) private() {}

// WithHeaders allows one to tell the driver to send additional HTTP
// headers with the payloads. Specifying headers like Content-Length,
// Content-Encoding and Content-Type may result in a broken driver.
Expand Down
17 changes: 17 additions & 0 deletions exporters/stdout/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,11 @@ func NewConfig(options ...Option) (Config, error) {
type Option interface {
// Apply option value to Config.
Apply(*Config)

// A private method to prevent users implementing the
// interface and so future additions to it will not
// violate compatibility.
private()
}

// WithWriter sets the export stream destination.
Expand All @@ -89,6 +94,8 @@ func (o writerOption) Apply(config *Config) {
config.Writer = o.W
}

func (writerOption) private() {}

// WithPrettyPrint sets the export stream format to use JSON.
func WithPrettyPrint() Option {
return prettyPrintOption(true)
Expand All @@ -100,6 +107,8 @@ func (o prettyPrintOption) Apply(config *Config) {
config.PrettyPrint = bool(o)
}

func (prettyPrintOption) private() {}

// WithoutTimestamps sets the export stream to not include timestamps.
func WithoutTimestamps() Option {
return timestampsOption(false)
Expand All @@ -111,6 +120,8 @@ func (o timestampsOption) Apply(config *Config) {
config.Timestamps = bool(o)
}

func (timestampsOption) private() {}

// WithLabelEncoder sets the label encoder used in export.
func WithLabelEncoder(enc attribute.Encoder) Option {
return labelEncoderOption{enc}
Expand All @@ -124,6 +135,8 @@ func (o labelEncoderOption) Apply(config *Config) {
config.LabelEncoder = o.LabelEncoder
}

func (labelEncoderOption) private() {}

// WithoutTraceExport disables all trace exporting.
func WithoutTraceExport() Option {
return disableTraceExportOption(true)
Expand All @@ -135,6 +148,8 @@ func (o disableTraceExportOption) Apply(config *Config) {
config.DisableTraceExport = bool(o)
}

func (disableTraceExportOption) private() {}

// WithoutMetricExport disables all metric exporting.
func WithoutMetricExport() Option {
return disableMetricExportOption(true)
Expand All @@ -145,3 +160,5 @@ type disableMetricExportOption bool
func (o disableMetricExportOption) Apply(config *Config) {
config.DisableMetricExport = bool(o)
}

func (disableMetricExportOption) private() {}
2 changes: 0 additions & 2 deletions internal/tools/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -540,7 +540,6 @@ golang.org/x/sys v0.0.0-20200323222414-85ca7c5b95cd/go.mod h1:h1NjWce9XRLGQEsW7w
golang.org/x/sys v0.0.0-20200519105757-fe76b779f299/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.0.0-20200602225109-6fdc65e7d980/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.0.0-20200930185726-fdedc70b468f/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.0.0-20210113181707-4bcb84eeeb78 h1:nVuTkr9L6Bq62qpUqKo/RnZCFfzDBL0bYo6w9OJUqZY=
golang.org/x/sys v0.0.0-20210113181707-4bcb84eeeb78/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.0.0-20210119212857-b64e53b001e4 h1:myAQVi0cGEoqQVR5POX+8RR2mrocKqNN1hmeMqhX27k=
golang.org/x/sys v0.0.0-20210119212857-b64e53b001e4/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
Expand Down Expand Up @@ -604,7 +603,6 @@ golang.org/x/tools v0.0.0-20201118003311-bd56c0adb394/go.mod h1:emZCQorbCU4vsT4f
golang.org/x/tools v0.0.0-20201230224404-63754364767c/go.mod h1:emZCQorbCU4vsT4fOWvOPXz4eW1wZW4PmDk9uLelYpA=
golang.org/x/tools v0.0.0-20210101214203-2dba1e4ea05c/go.mod h1:emZCQorbCU4vsT4fOWvOPXz4eW1wZW4PmDk9uLelYpA=
golang.org/x/tools v0.0.0-20210102185154-773b96fafca2/go.mod h1:emZCQorbCU4vsT4fOWvOPXz4eW1wZW4PmDk9uLelYpA=
golang.org/x/tools v0.0.0-20210106214847-113979e3529a h1:CB3a9Nez8M13wwlr/E2YtwoU+qYHKfC+JrDa45RXXoQ=
golang.org/x/tools v0.0.0-20210106214847-113979e3529a/go.mod h1:emZCQorbCU4vsT4fOWvOPXz4eW1wZW4PmDk9uLelYpA=
golang.org/x/tools v0.1.0 h1:po9/4sTYwZU9lPhi1tOrb4hCv3qrhiQ77LZfGa2OjwY=
golang.org/x/tools v0.1.0/go.mod h1:xkSsbof2nBLbhDlRMhhhyNLN/zl3eTqcnHD5viDpcZ0=
Expand Down
15 changes: 13 additions & 2 deletions oteltest/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,19 @@ func newConfig(opts ...Option) config {
// Option applies an option to a config.
type Option interface {
Apply(*config)

// A private method to prevent users implementing the
// interface and so future additions to it will not
// violate compatibility.
private()
}

type option struct{}

func (option) private() {}

type spanContextFuncOption struct {
option
SpanContextFunc func(context.Context) trace.SpanContext
}

Expand All @@ -76,10 +86,11 @@ func (o spanContextFuncOption) Apply(c *config) {
// WithSpanContextFunc sets the SpanContextFunc used to generate a new Spans
// context from a parent SpanContext.
func WithSpanContextFunc(f func(context.Context) trace.SpanContext) Option {
return spanContextFuncOption{f}
return spanContextFuncOption{SpanContextFunc: f}
}

type spanRecorderOption struct {
option
SpanRecorder *SpanRecorder
}

Expand All @@ -90,7 +101,7 @@ func (o spanRecorderOption) Apply(c *config) {
// WithSpanRecorder sets the SpanRecorder to use with the TracerProvider for
// testing.
func WithSpanRecorder(sr *SpanRecorder) Option {
return spanRecorderOption{sr}
return spanRecorderOption{SpanRecorder: sr}
}

// SpanRecorder performs operations to record a span as it starts and ends.
Expand Down
25 changes: 20 additions & 5 deletions sdk/resource/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,17 @@ type config struct {
type Option interface {
// Apply sets the Option value of a config.
Apply(*config)

// A private method to prevent users implementing the
// interface and so future additions to it will not
// violate compatibility.
private()
}

type option struct{}

func (option) private() {}

// WithAttributes adds attributes to the configured Resource.
func WithAttributes(attributes ...attribute.KeyValue) Option {
return WithDetectors(detectAttributes{attributes})
Expand All @@ -59,10 +68,11 @@ func (d detectAttributes) Detect(context.Context) (*Resource, error) {

// WithDetectors adds detectors to be evaluated for the configured resource.
func WithDetectors(detectors ...Detector) Option {
return detectorsOption{detectors}
return detectorsOption{detectors: detectors}
}

type detectorsOption struct {
option
detectors []Detector
}

Expand All @@ -74,10 +84,11 @@ func (o detectorsOption) Apply(cfg *config) {
// WithTelemetrySDK overrides the builtin `telemetry.sdk.*`
// attributes. Use nil to disable these attributes entirely.
func WithTelemetrySDK(d Detector) Option {
return telemetrySDKOption{d}
return telemetrySDKOption{Detector: d}
}

type telemetrySDKOption struct {
option
Detector
}

Expand All @@ -89,10 +100,11 @@ func (o telemetrySDKOption) Apply(cfg *config) {
// WithHost overrides the builtin `host.*` attributes. Use nil to
// disable these attributes entirely.
func WithHost(d Detector) Option {
return hostOption{d}
return hostOption{Detector: d}
}

type hostOption struct {
option
Detector
}

Expand All @@ -104,10 +116,11 @@ func (o hostOption) Apply(cfg *config) {
// WithFromEnv overrides the builtin detector for
// OTEL_RESOURCE_ATTRIBUTES. Use nil to disable environment checking.
func WithFromEnv(d Detector) Option {
return fromEnvOption{d}
return fromEnvOption{Detector: d}
}

type fromEnvOption struct {
option
Detector
}

Expand All @@ -122,7 +135,9 @@ func WithoutBuiltin() Option {
return noBuiltinOption{}
}

type noBuiltinOption struct{}
type noBuiltinOption struct {
option
}

// Apply implements Option.
func (o noBuiltinOption) Apply(cfg *config) {
Expand Down
13 changes: 13 additions & 0 deletions sdk/trace/sampling.go
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,11 @@ type config struct {
// ParentBasedSamplerOption configures the sampler for a particular sampling case.
type ParentBasedSamplerOption interface {
Apply(*config)

// A private method to prevent users implementing the
// interface and so future additions to it will not
// violate compatibility.
private()
}

// WithRemoteParentSampled sets the sampler for the case of sampled remote parent.
Expand All @@ -205,6 +210,8 @@ func (o remoteParentSampledOption) Apply(config *config) {
config.remoteParentSampled = o.s
}

func (remoteParentSampledOption) private() {}

// WithRemoteParentNotSampled sets the sampler for the case of remote parent
// which is not sampled.
func WithRemoteParentNotSampled(s Sampler) ParentBasedSamplerOption {
Expand All @@ -219,6 +226,8 @@ func (o remoteParentNotSampledOption) Apply(config *config) {
config.remoteParentNotSampled = o.s
}

func (remoteParentNotSampledOption) private() {}

// WithLocalParentSampled sets the sampler for the case of sampled local parent.
func WithLocalParentSampled(s Sampler) ParentBasedSamplerOption {
return localParentSampledOption{s}
Expand All @@ -232,6 +241,8 @@ func (o localParentSampledOption) Apply(config *config) {
config.localParentSampled = o.s
}

func (localParentSampledOption) private() {}

// WithLocalParentNotSampled sets the sampler for the case of local parent
// which is not sampled.
func WithLocalParentNotSampled(s Sampler) ParentBasedSamplerOption {
Expand All @@ -246,6 +257,8 @@ func (o localParentNotSampledOption) Apply(config *config) {
config.localParentNotSampled = o.s
}

func (localParentNotSampledOption) private() {}

func (pb parentBased) ShouldSample(p SamplingParameters) SamplingResult {
if p.ParentContext.IsValid() {
if p.HasRemoteParent {
Expand Down
7 changes: 7 additions & 0 deletions sdk/trace/span.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,11 @@ type ReadOnlySpan interface {
InstrumentationLibrary() instrumentation.Library
Resource() *resource.Resource
Snapshot() *export.SpanSnapshot

// A private method to prevent users implementing the
// interface and so future additions to it will not
// violate compatibility.
private()
}

// ReadWriteSpan exposes the same methods as trace.Span and in addition allows
Expand Down Expand Up @@ -505,6 +510,8 @@ func (s *span) addDroppedAttributeCount(delta int) {
atomic.AddInt64(&s.droppedAttributeCount, int64(delta))
}

func (*span) private() {}

func startSpanInternal(ctx context.Context, tr *tracer, name string, parent trace.SpanContext, remoteParent bool, o *trace.SpanConfig) *span {
span := &span{}
span.spanContext = parent
Expand Down
Loading

0 comments on commit 37688ef

Please sign in to comment.