From 37688ef67616aa5a99e27fb03320fd0dc87e590e Mon Sep 17 00:00:00 2001 From: Punya Biswal Date: Wed, 24 Feb 2021 13:03:35 -0500 Subject: [PATCH] revent 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 Co-authored-by: Tyler Yahn --- CHANGELOG.md | 15 +++++++++++++++ exporters/otlp/otlphttp/options.go | 23 +++++++++++++++++++++++ exporters/stdout/config.go | 17 +++++++++++++++++ internal/tools/go.sum | 2 -- oteltest/config.go | 15 +++++++++++++-- sdk/resource/config.go | 25 ++++++++++++++++++++----- sdk/trace/sampling.go | 13 +++++++++++++ sdk/trace/span.go | 7 +++++++ trace/config.go | 23 +++++++++++++++++++++++ 9 files changed, 131 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8ad2295f315..f39e2aefabd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/exporters/otlp/otlphttp/options.go b/exporters/otlp/otlphttp/options.go index f80ee474874..1948dae1395 100644 --- a/exporters/otlp/otlphttp/options.go +++ b/exporters/otlp/otlphttp/options.go @@ -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 @@ -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 @@ -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) @@ -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 { @@ -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 { @@ -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. @@ -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. @@ -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. @@ -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 { @@ -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. diff --git a/exporters/stdout/config.go b/exporters/stdout/config.go index 7700c20245f..994c245b098 100644 --- a/exporters/stdout/config.go +++ b/exporters/stdout/config.go @@ -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. @@ -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) @@ -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) @@ -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} @@ -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) @@ -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) @@ -145,3 +160,5 @@ type disableMetricExportOption bool func (o disableMetricExportOption) Apply(config *Config) { config.DisableMetricExport = bool(o) } + +func (disableMetricExportOption) private() {} diff --git a/internal/tools/go.sum b/internal/tools/go.sum index 32d846cf0c6..142b4fd6757 100644 --- a/internal/tools/go.sum +++ b/internal/tools/go.sum @@ -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= @@ -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= diff --git a/oteltest/config.go b/oteltest/config.go index e147eb03c3a..30a8798c4a9 100644 --- a/oteltest/config.go +++ b/oteltest/config.go @@ -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 } @@ -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 } @@ -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. diff --git a/sdk/resource/config.go b/sdk/resource/config.go index 6ea27f5a96f..0365caa26a8 100644 --- a/sdk/resource/config.go +++ b/sdk/resource/config.go @@ -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}) @@ -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 } @@ -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 } @@ -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 } @@ -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 } @@ -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) { diff --git a/sdk/trace/sampling.go b/sdk/trace/sampling.go index d6fda1c2a93..59c08369de1 100644 --- a/sdk/trace/sampling.go +++ b/sdk/trace/sampling.go @@ -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. @@ -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 { @@ -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} @@ -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 { @@ -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 { diff --git a/sdk/trace/span.go b/sdk/trace/span.go index 99de92c11ba..0bb4155b83f 100644 --- a/sdk/trace/span.go +++ b/sdk/trace/span.go @@ -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 @@ -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 diff --git a/trace/config.go b/trace/config.go index c320805901c..33335e7ac34 100644 --- a/trace/config.go +++ b/trace/config.go @@ -39,6 +39,11 @@ func NewTracerConfig(options ...TracerOption) *TracerConfig { // TracerOption applies an option to a TracerConfig. type TracerOption interface { ApplyTracer(*TracerConfig) + + // A private method to prevent users implementing the + // interface and so future additions to it will not + // violate compatibility. + private() } // SpanConfig is a group of options for a Span. @@ -74,6 +79,11 @@ func NewSpanConfig(options ...SpanOption) *SpanConfig { // SpanOption applies an option to a SpanConfig. type SpanOption interface { ApplySpan(*SpanConfig) + + // A private method to prevent users implementing the + // interface and so future additions to it will not + // violate compatibility. + private() } // NewEventConfig applies all the EventOptions to a returned SpanConfig. If no @@ -94,6 +104,11 @@ func NewEventConfig(options ...EventOption) *SpanConfig { // EventOption applies span event options to a SpanConfig. type EventOption interface { ApplyEvent(*SpanConfig) + + // A private method to prevent users implementing the + // interface and so future additions to it will not + // violate compatibility. + private() } // LifeCycleOption applies span life-cycle options to a SpanConfig. These @@ -108,6 +123,7 @@ type attributeSpanOption []attribute.KeyValue func (o attributeSpanOption) ApplySpan(c *SpanConfig) { o.apply(c) } func (o attributeSpanOption) ApplyEvent(c *SpanConfig) { o.apply(c) } +func (attributeSpanOption) private() {} func (o attributeSpanOption) apply(c *SpanConfig) { c.Attributes = append(c.Attributes, []attribute.KeyValue(o)...) } @@ -129,6 +145,7 @@ type timestampSpanOption time.Time func (o timestampSpanOption) ApplySpan(c *SpanConfig) { o.apply(c) } func (o timestampSpanOption) ApplyEvent(c *SpanConfig) { o.apply(c) } +func (timestampSpanOption) private() {} func (o timestampSpanOption) apply(c *SpanConfig) { c.Timestamp = time.Time(o) } // WithTimestamp sets the time of a Span life-cycle moment (e.g. started, @@ -140,6 +157,7 @@ func WithTimestamp(t time.Time) LifeCycleOption { type linksSpanOption []Link func (o linksSpanOption) ApplySpan(c *SpanConfig) { c.Links = append(c.Links, []Link(o)...) } +func (linksSpanOption) private() {} // WithLinks adds links to a Span. The links are added to the existing Span // links, i.e. this does not overwrite. @@ -150,6 +168,7 @@ func WithLinks(links ...Link) SpanOption { type recordSpanOption bool func (o recordSpanOption) ApplySpan(c *SpanConfig) { c.Record = bool(o) } +func (recordSpanOption) private() {} // WithRecord specifies that the span should be recorded. It is important to // note that implementations may override this option, i.e. if the span is a @@ -161,6 +180,7 @@ func WithRecord() SpanOption { type newRootSpanOption bool func (o newRootSpanOption) ApplySpan(c *SpanConfig) { c.NewRoot = bool(o) } +func (newRootSpanOption) private() {} // WithNewRoot specifies that the Span should be treated as a root Span. Any // existing parent span context will be ignored when defining the Span's trace @@ -172,6 +192,7 @@ func WithNewRoot() SpanOption { type spanKindSpanOption SpanKind func (o spanKindSpanOption) ApplySpan(c *SpanConfig) { c.SpanKind = SpanKind(o) } +func (o spanKindSpanOption) private() {} // WithSpanKind sets the SpanKind of a Span. func WithSpanKind(kind SpanKind) SpanOption { @@ -194,3 +215,5 @@ type instrumentationVersionOption string func (i instrumentationVersionOption) ApplyTracer(config *TracerConfig) { config.InstrumentationVersion = string(i) } + +func (instrumentationVersionOption) private() {}