diff --git a/CHANGELOG.md b/CHANGELOG.md index 87754dec6b3..30f32906f75 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,12 @@ ### 🛑 Breaking changes 🛑 +- Remove `Type` funcs in pdata (#4933) + +## v0.46.0 Beta + +### 🛑 Breaking changes 🛑 + - Deprecated funcs `config.DefaultConfig`, `confighttp.DefaultHTTPSettings`, `exporterhelper.DefaultTimeoutSettings`, `exporthelper.DefaultQueueSettings`, `exporterhelper.DefaultRetrySettings`, `testcomponents.DefaultFactories`, and `scraperhelper.DefaultScraperControllerSettings` in favour for their `NewDefault` method to adhere to contribution guidelines (#4865) @@ -30,9 +36,11 @@ - Deprecated `receiverhelper.WithMetrics` in favour of `component.WithMetricsReceiver` - Deprecated `receiverhelper.WithLogs` in favour of `component.WithLogsReceiver` - Deprecated `receiverhelper.NewFactory` in favour of `component.NewReceiverFactory` +- Change otel collector to enable open telemetry metrics through feature gate instead of a constant - Remove support for legacy otlp/http port. (#4916) - Remove deprecated funcs in pdata (#4809) -- Remove `Type` funcs in pdata (#4933) +- Remove deprecated Retrieve funcs/calls (#4922) +- Remove deprecated NewConfigProvider funcs (#4937) ### 💡 Enhancements 💡 @@ -46,6 +54,7 @@ that calls shutdown to terminate it; this is done per memory limiter instance. Added memory limiter factory to cache initiated object and be reused by similar config. This guarantees a single running `checkMemLimits` per config (#4886) +- Resolved race condition in collector when calling `Shutdown` (#4878) ## v0.45.0 Beta diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 363c8040f85..f906a96f23c 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -457,14 +457,6 @@ that each of the following steps is done in a separate version: 1. On `v0.N+2`, we change `func GetFoo() Foo` to `func GetFoo(context.Context) Foo` if desired or remove it entirely if needed. -#### Exceptions - -While the above is what we strive to follow, we acknowledge that some changes might be unfeasible to achieve in a -non-breaking manner. Exceptions to the outlined rules are acceptable if consensus can be obtained from approvers in the -pull request they are proposed. A reason for requesting the exception MUST be given in the pull request. Until unanimity -is obtained, approvers and maintainers are encouraged to discuss the issue at hand. If a consensus (unanimity) cannot be -obtained, the maintainers are then tasked in getting a decision using its regular means (voting, TC help, ...). - ## Updating Changelog An entry into the [Changelog](./CHANGELOG.md) is required for the following reasons: @@ -502,3 +494,12 @@ go: github.com/golangci/golangci-lint@v1.31.0 requires `go env GOPROXY` should return `https://proxy.golang.org,direct`. If it does not, set it as an environment variable: `export GOPROXY=https://proxy.golang.org,direct` + +## Exceptions + +While the rules in this and other documents in this repository is what we strive to follow, we acknowledge that rules may be +unfeasible to be applied in some situations. Exceptions to the rules +on this and other documents are acceptable if consensus can be obtained from approvers in the pull request they are proposed. +A reason for requesting the exception MUST be given in the pull request. Until unanimity is obtained, approvers and maintainers are +encouraged to discuss the issue at hand. If a consensus (unanimity) cannot be obtained, the maintainers are then tasked in getting a +decision using its regular means (voting, TC help, ...). diff --git a/README.md b/README.md index 09849d9cd04..12a395d26e0 100644 --- a/README.md +++ b/README.md @@ -111,6 +111,10 @@ Triagers ([@open-telemetry/collector-triagers](https://github.com/orgs/open-tele - [Punya Biswal](https://github.com/punya), Google - [Steve Flanders](https://github.com/flands), Splunk +Emeritus Triagers: + +- [Andrew Hsu](https://github.com/andrewhsu), Lightstep + Approvers ([@open-telemetry/collector-approvers](https://github.com/orgs/open-telemetry/teams/collector-approvers)): - [Anthony Mirabella](https://github.com/Aneurysm9), AWS @@ -118,12 +122,26 @@ Approvers ([@open-telemetry/collector-approvers](https://github.com/orgs/open-te - [Juraci Paixão Kröhling](https://github.com/jpkrohling), Grafana Labs - [Pablo Baeyens](https://github.com/mx-psi), DataDog +Emeritus Approvers: + +- [James Bebbington](https://github.com/james-bebbington), Google +- [Jay Camp](https://github.com/jrcamp), Splunk +- [Nail Islamov](https://github.com/nilebox), Google +- [Owais Lone](https://github.com/owais), Splunk +- [Rahul Patel](https://github.com/rghetia), Google +- [Steven Karis](https://github.com/sjkaris), Splunk +- [Yang Song](https://github.com/songy23), Google + Maintainers ([@open-telemetry/collector-maintainers](https://github.com/orgs/open-telemetry/teams/collector-maintainers)): - [Alex Boten](https://github.com/codeboten), Lightstep - [Bogdan Drutu](https://github.com/BogdanDrutu), Splunk - [Tigran Najaryan](https://github.com/tigrannajaryan), Splunk +Emeritus Maintainers: + +- [Paulo Janotti](https://github.com/pjanotti), Splunk + Learn more about roles in the [community repository](https://github.com/open-telemetry/community/blob/main/community-membership.md). Thanks to all the people who already contributed! diff --git a/cmd/builder/internal/builder/config.go b/cmd/builder/internal/builder/config.go index 77fbc7d50ad..43ce82feb48 100644 --- a/cmd/builder/internal/builder/config.go +++ b/cmd/builder/internal/builder/config.go @@ -25,7 +25,7 @@ import ( "go.uber.org/zap" ) -const defaultOtelColVersion = "0.45.0" +const defaultOtelColVersion = "0.46.0" // ErrInvalidGoMod indicates an invalid gomod var ErrInvalidGoMod = errors.New("invalid gomod specification for module") diff --git a/cmd/otelcorecol/builder-config.yaml b/cmd/otelcorecol/builder-config.yaml index 971ea967dc2..7f0a472c629 100644 --- a/cmd/otelcorecol/builder-config.yaml +++ b/cmd/otelcorecol/builder-config.yaml @@ -2,29 +2,29 @@ dist: module: go.opentelemetry.io/collector/cmd/otelcorecol name: otelcorecol description: Local OpenTelemetry Collector binary, testing only. - version: 0.45.1-dev - otelcol_version: 0.45.0 + version: 0.46.1-dev + otelcol_version: 0.46.0 receivers: - import: go.opentelemetry.io/collector/receiver/otlpreceiver - gomod: go.opentelemetry.io/collector v0.45.0 + gomod: go.opentelemetry.io/collector v0.46.0 exporters: - import: go.opentelemetry.io/collector/exporter/loggingexporter - gomod: go.opentelemetry.io/collector v0.45.0 + gomod: go.opentelemetry.io/collector v0.46.0 - import: go.opentelemetry.io/collector/exporter/otlpexporter - gomod: go.opentelemetry.io/collector v0.45.0 + gomod: go.opentelemetry.io/collector v0.46.0 - import: go.opentelemetry.io/collector/exporter/otlphttpexporter - gomod: go.opentelemetry.io/collector v0.45.0 + gomod: go.opentelemetry.io/collector v0.46.0 extensions: - import: go.opentelemetry.io/collector/extension/ballastextension - gomod: go.opentelemetry.io/collector v0.45.0 + gomod: go.opentelemetry.io/collector v0.46.0 - import: go.opentelemetry.io/collector/extension/zpagesextension - gomod: go.opentelemetry.io/collector v0.45.0 + gomod: go.opentelemetry.io/collector v0.46.0 processors: - import: go.opentelemetry.io/collector/processor/batchprocessor - gomod: go.opentelemetry.io/collector v0.45.0 + gomod: go.opentelemetry.io/collector v0.46.0 - import: go.opentelemetry.io/collector/processor/memorylimiterprocessor - gomod: go.opentelemetry.io/collector v0.45.0 + gomod: go.opentelemetry.io/collector v0.46.0 replaces: - go.opentelemetry.io/collector => ../../ diff --git a/cmd/otelcorecol/go.mod b/cmd/otelcorecol/go.mod index 0dd9ae87a80..b6483635868 100644 --- a/cmd/otelcorecol/go.mod +++ b/cmd/otelcorecol/go.mod @@ -6,7 +6,7 @@ go 1.17 require ( github.com/stretchr/testify v1.7.0 - go.opentelemetry.io/collector v0.45.0 + go.opentelemetry.io/collector v0.46.0 golang.org/x/sys v0.0.0-20220114195835-da31bd327af9 ) @@ -32,7 +32,7 @@ require ( github.com/klauspost/compress v1.14.4 // indirect github.com/knadh/koanf v1.4.0 // indirect github.com/lufia/plan9stats v0.0.0-20211012122336-39d0f177ccd0 // indirect - github.com/magiconair/properties v1.8.5 // indirect + github.com/magiconair/properties v1.8.6 // indirect github.com/matttproud/golang_protobuf_extensions v1.0.1 // indirect github.com/mitchellh/copystructure v1.2.0 // indirect github.com/mitchellh/mapstructure v1.4.3 // indirect @@ -54,7 +54,7 @@ require ( github.com/tklauser/numcpus v0.3.0 // indirect github.com/yusufpapurcu/wmi v1.2.2 // indirect go.opencensus.io v0.23.0 // indirect - go.opentelemetry.io/collector/model v0.45.0 // indirect + go.opentelemetry.io/collector/model v0.46.0 // indirect go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.29.0 // indirect go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.29.0 // indirect go.opentelemetry.io/contrib/zpages v0.29.0 // indirect diff --git a/cmd/otelcorecol/go.sum b/cmd/otelcorecol/go.sum index 7d7ea1d3b78..283a718c244 100644 --- a/cmd/otelcorecol/go.sum +++ b/cmd/otelcorecol/go.sum @@ -433,8 +433,9 @@ github.com/lufia/plan9stats v0.0.0-20211012122336-39d0f177ccd0 h1:6E+4a0GO5zZEnZ github.com/lufia/plan9stats v0.0.0-20211012122336-39d0f177ccd0/go.mod h1:zJYVVT2jmtg6P3p1VtQj7WsuWi/y4VnjVBn7F8KPB3I= github.com/lyft/protoc-gen-star v0.5.3 h1:zSGLzsUew8RT+ZKPHc3jnf8XLaVyHzTcAFBzHtCNR20= github.com/lyft/protoc-gen-star v0.5.3/go.mod h1:V0xaHgaf5oCCqmcxYcWiDfTiKsZsRc87/1qhoTACD8w= -github.com/magiconair/properties v1.8.5 h1:b6kJs+EmPFMYGkow9GiUyCyOvIwYetYJ3fSaWak/Gls= github.com/magiconair/properties v1.8.5/go.mod h1:y3VJvCyxH9uVvJTWEGAELF3aiYNyPKd5NZ3oSwXrF60= +github.com/magiconair/properties v1.8.6 h1:5ibWZ6iY0NctNGWo87LalDlEZ6R41TqbbDamhfG/Qzo= +github.com/magiconair/properties v1.8.6/go.mod h1:y3VJvCyxH9uVvJTWEGAELF3aiYNyPKd5NZ3oSwXrF60= github.com/mattn/go-colorable v0.0.9/go.mod h1:9vuHe8Xs5qXnSaW/c/ABM9alt+Vo+STaOChaDxuIBZU= github.com/mattn/go-colorable v0.1.4/go.mod h1:U0ppj6V5qS13XJ6of8GYAs25YV2eR4EVcfRqFIhoBtE= github.com/mattn/go-colorable v0.1.6/go.mod h1:u6P/XSegPjTcexA+o6vUJrdnUu04hMope9wVRipJSqc= diff --git a/cmd/otelcorecol/main.go b/cmd/otelcorecol/main.go index a560988945c..ad2a86a9aaf 100644 --- a/cmd/otelcorecol/main.go +++ b/cmd/otelcorecol/main.go @@ -21,7 +21,7 @@ func main() { info := component.BuildInfo{ Command: "otelcorecol", Description: "Local OpenTelemetry Collector binary, testing only.", - Version: "0.45.1-dev", + Version: "0.46.1-dev", } if err := run(service.CollectorSettings{BuildInfo: info, Factories: factories}); err != nil { diff --git a/config/configmapprovider/env_test.go b/config/configmapprovider/env_test.go index 237fca9a285..51569904287 100644 --- a/config/configmapprovider/env_test.go +++ b/config/configmapprovider/env_test.go @@ -62,14 +62,11 @@ func TestEnv(t *testing.T) { env := NewEnv() ret, err := env.Retrieve(context.Background(), envSchemePrefix+envName, nil) require.NoError(t, err) - cfg, err := ret.Get(context.Background()) - assert.NoError(t, err) expectedMap := config.NewMapFromStringMap(map[string]interface{}{ "processors::batch": nil, "exporters::otlp::endpoint": "localhost:4317", }) - assert.Equal(t, expectedMap.ToStringMap(), cfg.ToStringMap()) - assert.NoError(t, ret.Close(context.Background())) + assert.Equal(t, expectedMap.ToStringMap(), ret.Map.ToStringMap()) assert.NoError(t, env.Shutdown(context.Background())) } diff --git a/config/configmapprovider/expand_test.go b/config/configmapprovider/expand_test.go index f421ce7dd1e..5f427ffd8bf 100644 --- a/config/configmapprovider/expand_test.go +++ b/config/configmapprovider/expand_test.go @@ -120,6 +120,5 @@ func loadConfigMap(fileName string) (*config.Map, error) { if err != nil { return nil, err } - - return ret.Get(context.Background()) + return ret.Map, nil } diff --git a/config/configmapprovider/file_test.go b/config/configmapprovider/file_test.go index 56f5635f553..b6e26994022 100644 --- a/config/configmapprovider/file_test.go +++ b/config/configmapprovider/file_test.go @@ -64,14 +64,11 @@ func TestFile_RelativePath(t *testing.T) { fp := NewFile() ret, err := fp.Retrieve(context.Background(), fileSchemePrefix+filepath.Join("testdata", "default-config.yaml"), nil) require.NoError(t, err) - cfg, err := ret.Get(context.Background()) - assert.NoError(t, err) expectedMap := config.NewMapFromStringMap(map[string]interface{}{ "processors::batch": nil, "exporters::otlp::endpoint": "localhost:4317", }) - assert.Equal(t, expectedMap, cfg) - assert.NoError(t, ret.Close(context.Background())) + assert.Equal(t, expectedMap, ret.Map) assert.NoError(t, fp.Shutdown(context.Background())) } @@ -79,14 +76,11 @@ func TestFile_AbsolutePath(t *testing.T) { fp := NewFile() ret, err := fp.Retrieve(context.Background(), fileSchemePrefix+absolutePath(t, filepath.Join("testdata", "default-config.yaml")), nil) require.NoError(t, err) - cfg, err := ret.Get(context.Background()) - assert.NoError(t, err) expectedMap := config.NewMapFromStringMap(map[string]interface{}{ "processors::batch": nil, "exporters::otlp::endpoint": "localhost:4317", }) - assert.Equal(t, expectedMap, cfg) - assert.NoError(t, ret.Close(context.Background())) + assert.Equal(t, expectedMap, ret.Map) assert.NoError(t, fp.Shutdown(context.Background())) } diff --git a/config/configmapprovider/provider.go b/config/configmapprovider/provider.go index 614fcf94978..e18867f873f 100644 --- a/config/configmapprovider/provider.go +++ b/config/configmapprovider/provider.go @@ -97,12 +97,3 @@ type Retrieved struct { // // Should never be called concurrently with itself. type CloseFunc func(context.Context) error - -// Close calls the func only if not nil. -// Deprecated: [v0.45.0] Not needed, will be removed soon. You have access to the func. -func (f CloseFunc) Close(ctx context.Context) error { - if f == nil { - return nil - } - return f(ctx) -} diff --git a/config/configmapprovider/retrieved.go b/config/configmapprovider/retrieved.go deleted file mode 100644 index 3f58dae8120..00000000000 --- a/config/configmapprovider/retrieved.go +++ /dev/null @@ -1,72 +0,0 @@ -// Copyright The OpenTelemetry Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package configmapprovider // import "go.opentelemetry.io/collector/config/configmapprovider" - -import ( - "context" - "errors" - - "go.opentelemetry.io/collector/config" -) - -// Get returns the config Map. Should never be called after Close. -// Should never be called concurrently with itself or Close. -// -// Deprecated: [v0.45.0] Not needed, will be removed soon. You have access to the struct members. -func (r Retrieved) Get(ctx context.Context) (*config.Map, error) { - return r.Map, nil -} - -// GetFunc specifies the function invoked when the Retrieved.Get is being called. -// Deprecated: [v0.45.0] Not needed, will be removed soon. You have access to the struct members. -type GetFunc func(context.Context) (*config.Map, error) - -// Get implements the Retrieved.Get. -// Deprecated: [v0.45.0] Not needed, will be removed soon. You have access to the struct members. -func (f GetFunc) Get(ctx context.Context) (*config.Map, error) { - return f(ctx) -} - -// RetrievedOption represents the possible options for NewRetrieved. -// Deprecated: [v0.45.0] Not needed, will be removed soon. You have access to the struct members. -type RetrievedOption func(*Retrieved) - -// WithClose overrides the default `Close` function for a Retrieved. -// The default always returns nil. -// Deprecated: [v0.45.0] Not needed, will be removed soon. You have access to the struct members. -func WithClose(closeFunc CloseFunc) RetrievedOption { - return func(o *Retrieved) { - o.CloseFunc = closeFunc - } -} - -// NewRetrieved returns a Retrieved configured with the provided options. -// Deprecated: [v0.45.0] Not needed, will be removed soon. You have access to the struct members. -func NewRetrieved(getFunc GetFunc, options ...RetrievedOption) (Retrieved, error) { - if getFunc == nil { - return Retrieved{}, errors.New("nil getFunc") - } - cfg, err := getFunc(context.Background()) - if err != nil { - return Retrieved{}, err - } - ret := Retrieved{ - Map: cfg, - } - for _, op := range options { - op(&ret) - } - return ret, nil -} diff --git a/config/configmapprovider/retrieved_test.go b/config/configmapprovider/retrieved_test.go deleted file mode 100644 index 842f5d8ab45..00000000000 --- a/config/configmapprovider/retrieved_test.go +++ /dev/null @@ -1,60 +0,0 @@ -// Copyright The OpenTelemetry Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package configmapprovider - -import ( - "context" - "errors" - "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - - "go.opentelemetry.io/collector/config" -) - -func TestNewRetrieved_NilGetFunc(t *testing.T) { - _, err := NewRetrieved(nil) - assert.Error(t, err) -} - -func TestNewRetrieved_Default(t *testing.T) { - expectedCfg := config.NewMapFromStringMap(map[string]interface{}{"test": nil}) - ret, err := NewRetrieved(func(context.Context) (*config.Map, error) { return expectedCfg, nil }) - require.NoError(t, err) - cfg, err := ret.Get(context.Background()) - require.NoError(t, err) - assert.Equal(t, expectedCfg, cfg) - assert.NoError(t, ret.Close(context.Background())) -} - -func TestNewRetrieved_GetError(t *testing.T) { - expectedErr := errors.New("test") - _, err := NewRetrieved(func(context.Context) (*config.Map, error) { return nil, expectedErr }) - assert.Equal(t, expectedErr, err) -} - -func TestNewRetrieved_WithClose(t *testing.T) { - expectedCfg := config.NewMapFromStringMap(map[string]interface{}{"test": nil}) - expectedCloseErr := errors.New("test") - ret, err := NewRetrieved( - func(context.Context) (*config.Map, error) { return expectedCfg, nil }, - WithClose(func(ctx context.Context) error { return expectedCloseErr })) - require.NoError(t, err) - cfg, err := ret.Get(context.Background()) - require.NoError(t, err) - assert.Equal(t, expectedCfg, cfg) - assert.Equal(t, expectedCloseErr, ret.Close(context.Background())) -} diff --git a/config/configtelemetry/configtelemetry.go b/config/configtelemetry/configtelemetry.go index 01584f11cf4..4a1fdce7d1b 100644 --- a/config/configtelemetry/configtelemetry.go +++ b/config/configtelemetry/configtelemetry.go @@ -36,6 +36,8 @@ const ( levelDetailedStr = "detailed" ) +// Deprecated: UseOpenTelemetryForInternalMetrics has been deprecated. Uses feature flag in +// telemetry.useOtelForInternalMetrics to handle this feature const UseOpenTelemetryForInternalMetrics = false // Level is the level of internal telemetry (metrics, logs, traces about the component itself) diff --git a/go.mod b/go.mod index 13a430d2290..9f44cf7690c 100644 --- a/go.mod +++ b/go.mod @@ -11,7 +11,7 @@ require ( github.com/gorilla/mux v1.8.0 github.com/klauspost/compress v1.14.4 github.com/knadh/koanf v1.4.0 - github.com/magiconair/properties v1.8.5 + github.com/magiconair/properties v1.8.6 github.com/mitchellh/mapstructure v1.4.3 github.com/mostynb/go-grpc-compression v1.1.16 github.com/prometheus/common v0.32.1 @@ -21,7 +21,7 @@ require ( github.com/spf13/cobra v1.3.0 github.com/stretchr/testify v1.7.0 go.opencensus.io v0.23.0 - go.opentelemetry.io/collector/model v0.45.0 + go.opentelemetry.io/collector/model v0.46.0 go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.29.0 go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.29.0 go.opentelemetry.io/contrib/zpages v0.29.0 diff --git a/go.sum b/go.sum index 069a1aa44c6..17d93e9e27a 100644 --- a/go.sum +++ b/go.sum @@ -87,6 +87,7 @@ github.com/cenkalti/backoff/v4 v4.1.2 h1:6Yo7N8UP2K6LWZnW94DLVSSrbobcWdVzAYOisuD github.com/cenkalti/backoff/v4 v4.1.2/go.mod h1:scbssz8iZGpm3xbr14ovlUdkxfGXNInqkPWOWmG2CLw= github.com/census-instrumentation/opencensus-proto v0.2.1/go.mod h1:f6KPmirojxKA12rnyqOA5BBL4O983OfeGPqjHWSTneU= github.com/census-instrumentation/opencensus-proto v0.3.0/go.mod h1:f6KPmirojxKA12rnyqOA5BBL4O983OfeGPqjHWSTneU= +github.com/cespare/xxhash v1.1.0 h1:a6HrQnmkObjyL+Gs60czilIUGqrzKutQD6XZog3p+ko= github.com/cespare/xxhash v1.1.0/go.mod h1:XrSqR1VqqWfGrhpAt58auRo0WTKS1nRRg3ghfAqPWnc= github.com/cespare/xxhash/v2 v2.1.1/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs= github.com/cespare/xxhash/v2 v2.1.2 h1:YRXhKfTDauu4ajMg1TPgFO5jnlC2HCbmLXMcTG5cbYE= @@ -330,8 +331,9 @@ github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE= github.com/lufia/plan9stats v0.0.0-20211012122336-39d0f177ccd0 h1:6E+4a0GO5zZEnZ81pIr0yLvtUWk2if982qA3F3QD6H4= github.com/lufia/plan9stats v0.0.0-20211012122336-39d0f177ccd0/go.mod h1:zJYVVT2jmtg6P3p1VtQj7WsuWi/y4VnjVBn7F8KPB3I= github.com/lyft/protoc-gen-star v0.5.3/go.mod h1:V0xaHgaf5oCCqmcxYcWiDfTiKsZsRc87/1qhoTACD8w= -github.com/magiconair/properties v1.8.5 h1:b6kJs+EmPFMYGkow9GiUyCyOvIwYetYJ3fSaWak/Gls= github.com/magiconair/properties v1.8.5/go.mod h1:y3VJvCyxH9uVvJTWEGAELF3aiYNyPKd5NZ3oSwXrF60= +github.com/magiconair/properties v1.8.6 h1:5ibWZ6iY0NctNGWo87LalDlEZ6R41TqbbDamhfG/Qzo= +github.com/magiconair/properties v1.8.6/go.mod h1:y3VJvCyxH9uVvJTWEGAELF3aiYNyPKd5NZ3oSwXrF60= github.com/mattn/go-colorable v0.0.9/go.mod h1:9vuHe8Xs5qXnSaW/c/ABM9alt+Vo+STaOChaDxuIBZU= github.com/mattn/go-colorable v0.1.4/go.mod h1:U0ppj6V5qS13XJ6of8GYAs25YV2eR4EVcfRqFIhoBtE= github.com/mattn/go-colorable v0.1.6/go.mod h1:u6P/XSegPjTcexA+o6vUJrdnUu04hMope9wVRipJSqc= diff --git a/model/pdata/logs.go b/model/pdata/logs.go index 212097f18ed..75982879c9d 100644 --- a/model/pdata/logs.go +++ b/model/pdata/logs.go @@ -65,6 +65,13 @@ func (ld Logs) InternalRep() internal.LogsWrapper { return internal.LogsFromOtlp(ld.orig) } +// MoveTo moves all properties from the current struct to dest +// resetting the current instance to its zero value. +func (ld Logs) MoveTo(dest Logs) { + *dest.orig = *ld.orig + *ld.orig = otlplogs.LogsData{} +} + // Clone returns a copy of Logs. func (ld Logs) Clone() Logs { cloneLd := NewLogs() diff --git a/model/pdata/logs_test.go b/model/pdata/logs_test.go index 825d385d5cb..929e6d32d11 100644 --- a/model/pdata/logs_test.go +++ b/model/pdata/logs_test.go @@ -17,26 +17,29 @@ package pdata import ( "testing" + gogoproto "github.com/gogo/protobuf/proto" "github.com/stretchr/testify/assert" + goproto "google.golang.org/protobuf/proto" + "google.golang.org/protobuf/types/known/emptypb" "go.opentelemetry.io/collector/model/internal" otlplogs "go.opentelemetry.io/collector/model/internal/data/protogen/logs/v1" ) func TestLogRecordCount(t *testing.T) { - md := NewLogs() - assert.EqualValues(t, 0, md.LogRecordCount()) + logs := NewLogs() + assert.EqualValues(t, 0, logs.LogRecordCount()) - rl := md.ResourceLogs().AppendEmpty() - assert.EqualValues(t, 0, md.LogRecordCount()) + rl := logs.ResourceLogs().AppendEmpty() + assert.EqualValues(t, 0, logs.LogRecordCount()) ill := rl.InstrumentationLibraryLogs().AppendEmpty() - assert.EqualValues(t, 0, md.LogRecordCount()) + assert.EqualValues(t, 0, logs.LogRecordCount()) ill.LogRecords().AppendEmpty() - assert.EqualValues(t, 1, md.LogRecordCount()) + assert.EqualValues(t, 1, logs.LogRecordCount()) - rms := md.ResourceLogs() + rms := logs.ResourceLogs() rms.EnsureCapacity(3) rms.AppendEmpty().InstrumentationLibraryLogs().AppendEmpty() illl := rms.AppendEmpty().InstrumentationLibraryLogs().AppendEmpty().LogRecords() @@ -44,7 +47,7 @@ func TestLogRecordCount(t *testing.T) { illl.AppendEmpty() } // 5 + 1 (from rms.At(0) initialized first) - assert.EqualValues(t, 6, md.LogRecordCount()) + assert.EqualValues(t, 6, logs.LogRecordCount()) } func TestLogRecordCountWithEmpty(t *testing.T) { @@ -74,9 +77,51 @@ func TestLogRecordCountWithEmpty(t *testing.T) { func TestToFromLogProto(t *testing.T) { wrapper := internal.LogsFromOtlp(&otlplogs.LogsData{}) - ld := LogsFromInternalRep(wrapper) - assert.EqualValues(t, NewLogs(), ld) - assert.EqualValues(t, &otlplogs.LogsData{}, ld.orig) + logs := LogsFromInternalRep(wrapper) + assert.EqualValues(t, NewLogs(), logs) + assert.EqualValues(t, &otlplogs.LogsData{}, logs.orig) +} + +func TestResourceLogsWireCompatibility(t *testing.T) { + // This test verifies that OTLP ProtoBufs generated using goproto lib in + // opentelemetry-proto repository OTLP ProtoBufs generated using gogoproto lib in + // this repository are wire compatible. + + // Generate ResourceLogs as pdata struct. + logs := generateTestResourceLogs() + + // Marshal its underlying ProtoBuf to wire. + wire1, err := gogoproto.Marshal(logs.orig) + assert.NoError(t, err) + assert.NotNil(t, wire1) + + // Unmarshal from the wire to OTLP Protobuf in goproto's representation. + var goprotoMessage emptypb.Empty + err = goproto.Unmarshal(wire1, &goprotoMessage) + assert.NoError(t, err) + + // Marshal to the wire again. + wire2, err := goproto.Marshal(&goprotoMessage) + assert.NoError(t, err) + assert.NotNil(t, wire2) + + // Unmarshal from the wire into gogoproto's representation. + var gogoprotoRS2 otlplogs.ResourceLogs + err = gogoproto.Unmarshal(wire2, &gogoprotoRS2) + assert.NoError(t, err) + + // Now compare that the original and final ProtoBuf messages are the same. + // This proves that goproto and gogoproto marshaling/unmarshaling are wire compatible. + assert.EqualValues(t, logs.orig, &gogoprotoRS2) +} + +func TestLogsMoveTo(t *testing.T) { + logs := NewLogs() + fillTestResourceLogsSlice(logs.ResourceLogs()) + dest := NewLogs() + logs.MoveTo(dest) + assert.EqualValues(t, NewLogs(), logs) + assert.EqualValues(t, generateTestResourceLogsSlice(), dest.ResourceLogs()) } func TestLogsClone(t *testing.T) { diff --git a/model/pdata/metrics.go b/model/pdata/metrics.go index 341293562c8..a7637a7a64f 100644 --- a/model/pdata/metrics.go +++ b/model/pdata/metrics.go @@ -70,6 +70,13 @@ func (md Metrics) Clone() Metrics { return cloneMd } +// MoveTo moves all properties from the current struct to dest +// resetting the current instance to its zero value. +func (md Metrics) MoveTo(dest Metrics) { + *dest.orig = *md.orig + *md.orig = otlpmetrics.MetricsData{} +} + // ResourceMetrics returns the ResourceMetricsSlice associated with this Metrics. func (md Metrics) ResourceMetrics() ResourceMetricsSlice { return newResourceMetricsSlice(&md.orig.ResourceMetrics) diff --git a/model/pdata/metrics_test.go b/model/pdata/metrics_test.go index bfde1459250..e9d92696cb4 100644 --- a/model/pdata/metrics_test.go +++ b/model/pdata/metrics_test.go @@ -56,10 +56,10 @@ func TestResourceMetricsWireCompatibility(t *testing.T) { // this repository are wire compatible. // Generate ResourceMetrics as pdata struct. - pdataRM := generateTestResourceMetrics() + metrics := generateTestResourceMetrics() // Marshal its underlying ProtoBuf to wire. - wire1, err := gogoproto.Marshal(pdataRM.orig) + wire1, err := gogoproto.Marshal(metrics.orig) assert.NoError(t, err) assert.NotNil(t, wire1) @@ -80,7 +80,7 @@ func TestResourceMetricsWireCompatibility(t *testing.T) { // Now compare that the original and final ProtoBuf messages are the same. // This proves that goproto and gogoproto marshaling/unmarshaling are wire compatible. - assert.True(t, assert.EqualValues(t, pdataRM.orig, &gogoprotoRM)) + assert.True(t, assert.EqualValues(t, metrics.orig, &gogoprotoRM)) } func TestMetricCount(t *testing.T) { @@ -192,6 +192,15 @@ func TestDataPointCountWithNilDataPoints(t *testing.T) { assert.EqualValues(t, 0, metrics.DataPointCount()) } +func TestMetricsMoveTo(t *testing.T) { + metrics := NewMetrics() + fillTestResourceMetricsSlice(metrics.ResourceMetrics()) + dest := NewMetrics() + metrics.MoveTo(dest) + assert.EqualValues(t, NewMetrics(), metrics) + assert.EqualValues(t, generateTestResourceMetricsSlice(), dest.ResourceMetrics()) +} + func TestOtlpToInternalReadOnly(t *testing.T) { md := Metrics{orig: &otlpmetrics.MetricsData{ ResourceMetrics: []*otlpmetrics.ResourceMetrics{ diff --git a/model/pdata/traces.go b/model/pdata/traces.go index 2cc3fc7faac..30196cd3a1c 100644 --- a/model/pdata/traces.go +++ b/model/pdata/traces.go @@ -63,6 +63,13 @@ func (td Traces) InternalRep() internal.TracesWrapper { return internal.TracesFromOtlp(td.orig) } +// MoveTo moves all properties from the current struct to dest +// resetting the current instance to its zero value. +func (td Traces) MoveTo(dest Traces) { + *dest.orig = *td.orig + *td.orig = otlptrace.TracesData{} +} + // Clone returns a copy of Traces. func (td Traces) Clone() Traces { cloneTd := NewTraces() diff --git a/model/pdata/traces_test.go b/model/pdata/traces_test.go index 67a576a0fa6..b6be7b258d2 100644 --- a/model/pdata/traces_test.go +++ b/model/pdata/traces_test.go @@ -27,19 +27,19 @@ import ( ) func TestSpanCount(t *testing.T) { - md := NewTraces() - assert.EqualValues(t, 0, md.SpanCount()) + traces := NewTraces() + assert.EqualValues(t, 0, traces.SpanCount()) - rs := md.ResourceSpans().AppendEmpty() - assert.EqualValues(t, 0, md.SpanCount()) + rs := traces.ResourceSpans().AppendEmpty() + assert.EqualValues(t, 0, traces.SpanCount()) ils := rs.InstrumentationLibrarySpans().AppendEmpty() - assert.EqualValues(t, 0, md.SpanCount()) + assert.EqualValues(t, 0, traces.SpanCount()) ils.Spans().AppendEmpty() - assert.EqualValues(t, 1, md.SpanCount()) + assert.EqualValues(t, 1, traces.SpanCount()) - rms := md.ResourceSpans() + rms := traces.ResourceSpans() rms.EnsureCapacity(3) rms.AppendEmpty().InstrumentationLibrarySpans().AppendEmpty() ilss := rms.AppendEmpty().InstrumentationLibrarySpans().AppendEmpty().Spans() @@ -47,7 +47,7 @@ func TestSpanCount(t *testing.T) { ilss.AppendEmpty() } // 5 + 1 (from rms.At(0) initialized first) - assert.EqualValues(t, 6, md.SpanCount()) + assert.EqualValues(t, 6, traces.SpanCount()) } func TestSpanCountWithEmpty(t *testing.T) { @@ -76,9 +76,9 @@ func TestSpanCountWithEmpty(t *testing.T) { func TestToFromOtlp(t *testing.T) { otlp := &otlptrace.TracesData{} - td := TracesFromInternalRep(internal.TracesFromOtlp(otlp)) - assert.EqualValues(t, NewTraces(), td) - assert.EqualValues(t, otlp, internal.TracesToOtlp(td.InternalRep())) + traces := TracesFromInternalRep(internal.TracesFromOtlp(otlp)) + assert.EqualValues(t, NewTraces(), traces) + assert.EqualValues(t, otlp, internal.TracesToOtlp(traces.InternalRep())) // More tests in ./tracedata/traces_test.go. Cannot have them here because of // circular dependency. } @@ -89,10 +89,10 @@ func TestResourceSpansWireCompatibility(t *testing.T) { // this repository are wire compatible. // Generate ResourceSpans as pdata struct. - pdataRS := generateTestResourceSpans() + traces := generateTestResourceSpans() // Marshal its underlying ProtoBuf to wire. - wire1, err := gogoproto.Marshal(pdataRS.orig) + wire1, err := gogoproto.Marshal(traces.orig) assert.NoError(t, err) assert.NotNil(t, wire1) @@ -113,7 +113,16 @@ func TestResourceSpansWireCompatibility(t *testing.T) { // Now compare that the original and final ProtoBuf messages are the same. // This proves that goproto and gogoproto marshaling/unmarshaling are wire compatible. - assert.EqualValues(t, pdataRS.orig, &gogoprotoRS2) + assert.EqualValues(t, traces.orig, &gogoprotoRS2) +} + +func TestTracesMoveTo(t *testing.T) { + traces := NewTraces() + fillTestResourceSpansSlice(traces.ResourceSpans()) + dest := NewTraces() + traces.MoveTo(dest) + assert.EqualValues(t, NewTraces(), traces) + assert.EqualValues(t, generateTestResourceSpansSlice(), dest.ResourceSpans()) } func TestTracesClone(t *testing.T) { diff --git a/service/collector.go b/service/collector.go index ace363a84ca..5e54de3da6a 100644 --- a/service/collector.go +++ b/service/collector.go @@ -50,6 +50,20 @@ const ( Closed ) +func (s State) String() string { + switch s { + case Starting: + return "Starting" + case Running: + return "Running" + case Closing: + return "Closing" + case Closed: + return "Closed" + } + return "UNKNOWN" +} + // (Internal note) Collector Lifecycle: // - New constructs a new Collector. // - Run starts the collector. @@ -96,6 +110,8 @@ func New(set CollectorSettings) (*Collector, error) { set: set, state: state, + + shutdownChan: make(chan struct{}), }, nil } @@ -113,12 +129,14 @@ func (col *Collector) GetLogger() *zap.Logger { // Shutdown shuts down the collector server. func (col *Collector) Shutdown() { - defer func() { - if r := recover(); r != nil { - col.logger.Info("shutdownChan already closed") - } - }() - close(col.shutdownChan) + // Only shutdown if we're in a Running or Starting State else noop + state := col.GetState() + if state == Running || state == Starting { + defer func() { + recover() // nolint:errcheck + }() + close(col.shutdownChan) + } } // runAndWaitForShutdownEvent waits for one of the shutdown events that can happen. @@ -131,7 +149,6 @@ func (col *Collector) runAndWaitForShutdownEvent(ctx context.Context) error { signal.Notify(col.signalsChannel, os.Interrupt, syscall.SIGTERM) } - col.shutdownChan = make(chan struct{}) col.setCollectorState(Running) LOOP: for { diff --git a/service/collector_test.go b/service/collector_test.go index 27a80f1bb1d..56b9d3a4bef 100644 --- a/service/collector_test.go +++ b/service/collector_test.go @@ -38,8 +38,16 @@ import ( "go.opentelemetry.io/collector/component" "go.opentelemetry.io/collector/internal/testcomponents" "go.opentelemetry.io/collector/internal/testutil" + "go.opentelemetry.io/collector/service/featuregate" ) +func TestStateString(t *testing.T) { + assert.Equal(t, "Starting", Starting.String()) + assert.Equal(t, "Running", Running.String()) + assert.Equal(t, "Closing", Closing.String()) + assert.Equal(t, "Closed", Closed.String()) +} + // TestCollector_StartAsGoRoutine must be the first unit test on the file, // to test for initialization without setting CLI flags. func TestCollector_StartAsGoRoutine(t *testing.T) { @@ -80,7 +88,7 @@ func TestCollector_StartAsGoRoutine(t *testing.T) { }, time.Second*2, time.Millisecond*200) } -func TestCollector_Start(t *testing.T) { +func testCollectorStartHelper(t *testing.T) { factories, err := testcomponents.NewDefaultFactories() require.NoError(t, err) var once sync.Once @@ -135,6 +143,31 @@ func TestCollector_Start(t *testing.T) { }, time.Second*2, time.Millisecond*200) } +// as telemetry instance is initialized only once, we need to reset it before each test so the metrics endpoint can +// have correct handler spawned +func resetCollectorTelemetry() { + collectorTelemetry = &colTelemetry{} +} + +func TestCollector_Start(t *testing.T) { + resetCollectorTelemetry() + testCollectorStartHelper(t) +} + +func TestCollector_StartWithOtelInternalMetrics(t *testing.T) { + resetCollectorTelemetry() + originalFlag := featuregate.IsEnabled(useOtelForInternalMetricsfeatureGateID) + defer func() { + featuregate.Apply(map[string]bool{ + useOtelForInternalMetricsfeatureGateID: originalFlag, + }) + }() + featuregate.Apply(map[string]bool{ + useOtelForInternalMetricsfeatureGateID: true, + }) + testCollectorStartHelper(t) +} + // TestCollector_ShutdownNoop verifies that shutdown can be called even if a collector // has yet to be started and it will execute without error. func TestCollector_ShutdownNoop(t *testing.T) { @@ -153,6 +186,42 @@ func TestCollector_ShutdownNoop(t *testing.T) { require.NotPanics(t, func() { col.Shutdown() }) } +func TestCollector_ShutdownBeforeRun(t *testing.T) { + // use a mock AppTelemetry struct to return an error on shutdown + preservedAppTelemetry := collectorTelemetry + collectorTelemetry = &colTelemetry{} + defer func() { collectorTelemetry = preservedAppTelemetry }() + + factories, err := testcomponents.NewDefaultFactories() + require.NoError(t, err) + + set := CollectorSettings{ + BuildInfo: component.NewDefaultBuildInfo(), + Factories: factories, + ConfigProvider: MustNewDefaultConfigProvider([]string{filepath.Join("testdata", "otelcol-config.yaml")}, nil), + } + col, err := New(set) + require.NoError(t, err) + + // Calling shutdown before collector is running should cause it to return quickly + col.Shutdown() + + colDone := make(chan struct{}) + go func() { + defer close(colDone) + colErr := col.Run(context.Background()) + if colErr != nil { + err = colErr + } + }() + + col.Shutdown() + <-colDone + assert.Eventually(t, func() bool { + return Closed == col.GetState() + }, time.Second*2, time.Millisecond*200) +} + type mockColTelemetry struct{} func (tel *mockColTelemetry) init(*Collector) error { diff --git a/service/config_provider.go b/service/config_provider.go index 6ac4541fe6c..326b58eec89 100644 --- a/service/config_provider.go +++ b/service/config_provider.go @@ -81,26 +81,7 @@ type configProvider struct { // * Then unmarshalls the final config.Config using the given configunmarshaler.ConfigUnmarshaler. // // The `configMapProviders` is a map of pairs . -// -// Notice: This API is experimental. func MustNewConfigProvider( - locations []string, - configMapProviders map[string]configmapprovider.Provider, - cfgMapConverters []config.MapConverterFunc, - configUnmarshaler configunmarshaler.ConfigUnmarshaler) ConfigProvider { - return NewConfigProvider(locations, configMapProviders, cfgMapConverters, configUnmarshaler) -} - -// Deprecated: [v0.44.0] use MustNewConfigProvider instead -// NewConfigProvider returns a new ConfigProvider that provides the configuration: -// * Retrieve the config.Map by merging all retrieved maps from all the configmapprovider.Provider in order. -// * Then applies all the config.MapConverterFunc in the given order. -// * Then unmarshalls the final config.Config using the given configunmarshaler.ConfigUnmarshaler. -// -// The `configMapProviders` is a map of pairs . -// -// Notice: This API is experimental. -func NewConfigProvider( locations []string, configMapProviders map[string]configmapprovider.Provider, cfgMapConverters []config.MapConverterFunc, @@ -120,14 +101,7 @@ func NewConfigProvider( // MustNewDefaultConfigProvider returns the default ConfigProvider, and it creates configuration from a file // defined by the given configFile and overwrites fields using properties. func MustNewDefaultConfigProvider(configLocations []string, properties []string) ConfigProvider { - return NewDefaultConfigProvider(configLocations, properties) -} - -// Deprecated: [v.0.44.0] use MustNewDefaultConfigProvider instead -// NewDefaultConfigProvider returns the default ConfigProvider, and it creates configuration from a file -// defined by the given configFile and overwrites fields using properties. -func NewDefaultConfigProvider(configLocations []string, properties []string) ConfigProvider { - return NewConfigProvider( + return MustNewConfigProvider( configLocations, map[string]configmapprovider.Provider{ "file": configmapprovider.NewFile(), diff --git a/service/telemetry.go b/service/telemetry.go index 88e17fe3aa7..daed07bd04b 100644 --- a/service/telemetry.go +++ b/service/telemetry.go @@ -38,6 +38,7 @@ import ( "go.opentelemetry.io/collector/internal/version" semconv "go.opentelemetry.io/collector/model/semconv/v1.5.0" "go.opentelemetry.io/collector/processor/batchprocessor" + "go.opentelemetry.io/collector/service/featuregate" telemetry2 "go.opentelemetry.io/collector/service/internal/telemetry" ) @@ -50,8 +51,21 @@ const AddCollectorVersionTag = true const ( zapKeyTelemetryAddress = "address" zapKeyTelemetryLevel = "level" + + // useOtelForInternalMetricsfeatureGateID is the feature gate ID that controls whether the collector uses open + // telemetry for internal metrics. + useOtelForInternalMetricsfeatureGateID = "telemetry.useOtelForInternalMetrics" ) +func init() { + //nolint:staticcheck + featuregate.Register(featuregate.Gate{ + ID: useOtelForInternalMetricsfeatureGateID, + Description: "controls whether the collector to uses open telemetry for internal metrics", + Enabled: configtelemetry.UseOpenTelemetryForInternalMetrics, + }) +} + type collectorTelemetryExporter interface { init(col *Collector) error shutdown() error @@ -98,7 +112,7 @@ func (tel *colTelemetry) initOnce(col *Collector) error { instanceID := instanceUUID.String() var pe http.Handler - if configtelemetry.UseOpenTelemetryForInternalMetrics { + if featuregate.IsEnabled(useOtelForInternalMetricsfeatureGateID) { otelHandler, err := tel.initOpenTelemetry() if err != nil { return err diff --git a/versions.yaml b/versions.yaml index d19d854f0b1..18167be44df 100644 --- a/versions.yaml +++ b/versions.yaml @@ -14,7 +14,7 @@ module-sets: collector-base: - version: v0.45.0 + version: v0.46.0 modules: - go.opentelemetry.io/collector - go.opentelemetry.io/collector/cmd/builder