From 268cdc29992f0850daedd3cf4cc18d5f79a34a78 Mon Sep 17 00:00:00 2001 From: Sanket Teli <104385297+Sanket-0510@users.noreply.github.com> Date: Thu, 21 Mar 2024 02:57:48 +0530 Subject: [PATCH 1/5] [confignet] added NewDefaultFunctions (#9671) **Description:** Added newDefault methods for structs in confignet package **Link to tracking Issue:** closes #9656 **Testing:** Tests were added for the newDefault functions **Documentation:** godoc --------- Co-authored-by: Tyler Helmuth <12352919+TylerHelmuth@users.noreply.github.com> --- .chloggen/confignet-add-newdefault-func.yaml | 25 ++++++++++++++++++++ config/confignet/confignet.go | 19 +++++++++++++++ config/confignet/confignet_test.go | 18 ++++++++++++++ 3 files changed, 62 insertions(+) create mode 100644 .chloggen/confignet-add-newdefault-func.yaml diff --git a/.chloggen/confignet-add-newdefault-func.yaml b/.chloggen/confignet-add-newdefault-func.yaml new file mode 100644 index 00000000000..373fc89b299 --- /dev/null +++ b/.chloggen/confignet-add-newdefault-func.yaml @@ -0,0 +1,25 @@ +# Use this changelog template to create an entry for release notes. + +# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' +change_type: enhancement + +# The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver) +component: confignet + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: Adds `NewDefault*` functions for all the config structs. + +# One or more tracking issues or pull requests related to the change +issues: [9656] + +# (Optional) One or more lines of additional information to render under the primary note. +# These lines will be padded with 2 spaces and then inserted directly into the document. +# Use pipe (|) for multiline entries. +subtext: + +# Optional: The change log or logs in which this entry should be included. +# e.g. '[user]' or '[user, api]' +# Include 'user' if the change is relevant to end users. +# Include 'api' if there is a change to a library API. +# Default: '[user]' +change_logs: [api] diff --git a/config/confignet/confignet.go b/config/confignet/confignet.go index 23cb4f78ce0..0f34aebb69f 100644 --- a/config/confignet/confignet.go +++ b/config/confignet/confignet.go @@ -62,6 +62,11 @@ type DialerConfig struct { Timeout time.Duration `mapstructure:"timeout"` } +// NewDefaultDialerConfig creates a new DialerConfig with any default values set +func NewDefaultDialerConfig() DialerConfig { + return DialerConfig{} +} + // AddrConfig represents a network endpoint address. type AddrConfig struct { // Endpoint configures the address for this network connection. @@ -79,6 +84,13 @@ type AddrConfig struct { DialerConfig DialerConfig `mapstructure:"dialer"` } +// NewDefaultAddrConfig creates a new AddrConfig with any default values set +func NewDefaultAddrConfig() AddrConfig { + return AddrConfig{ + DialerConfig: NewDefaultDialerConfig(), + } +} + // Dial equivalent with net.Dialer's DialContext for this address. func (na *AddrConfig) Dial(ctx context.Context) (net.Conn, error) { d := net.Dialer{Timeout: na.DialerConfig.Timeout} @@ -124,6 +136,13 @@ type TCPAddrConfig struct { DialerConfig DialerConfig `mapstructure:"dialer"` } +// NewDefaultTCPAddrConfig creates a new TCPAddrConfig with any default values set +func NewDefaultTCPAddrConfig() TCPAddrConfig { + return TCPAddrConfig{ + DialerConfig: NewDefaultDialerConfig(), + } +} + // Dial equivalent with net.Dialer's DialContext for this address. func (na *TCPAddrConfig) Dial(ctx context.Context) (net.Conn, error) { d := net.Dialer{Timeout: na.DialerConfig.Timeout} diff --git a/config/confignet/confignet_test.go b/config/confignet/confignet_test.go index 22eef303132..9375561dc39 100644 --- a/config/confignet/confignet_test.go +++ b/config/confignet/confignet_test.go @@ -14,6 +14,24 @@ import ( "github.com/stretchr/testify/require" ) +func TestNewDefaultDialerConfig(t *testing.T) { + expectedDialerConfig := DialerConfig{} + dialerConfig := NewDefaultDialerConfig() + require.Equal(t, expectedDialerConfig, dialerConfig) +} + +func TestNewDefaultAddrConfig(t *testing.T) { + expectedAddrConfig := AddrConfig{} + addrConfig := NewDefaultAddrConfig() + require.Equal(t, expectedAddrConfig, addrConfig) +} + +func TestNewDefaultTCPAddrConfig(t *testing.T) { + expectedTCPAddrConfig := TCPAddrConfig{} + tcpAddrconfig := NewDefaultTCPAddrConfig() + require.Equal(t, expectedTCPAddrConfig, tcpAddrconfig) +} + func TestAddrConfigTimeout(t *testing.T) { nac := &AddrConfig{ Endpoint: "localhost:0", From fc4c13d3c2822bec39fa9d9658836d1a020c6844 Mon Sep 17 00:00:00 2001 From: Tyler Helmuth <12352919+TylerHelmuth@users.noreply.github.com> Date: Wed, 20 Mar 2024 15:28:45 -0600 Subject: [PATCH 2/5] [configgrpc] Remove deprecated func, add ToServer with context (#9787) **Description:** Removes deprecated `ToServer`. Deprecate `ToServerContext` Add new `ToServer` with `context.Context`. **Link to tracking Issue:** Related to https://github.com/open-telemetry/opentelemetry-collector/issues/9490 --------- Co-authored-by: Dmitrii Anoshin --- .../configgrpc-remove-deprecated-func.yaml | 25 +++++++++++++++++++ .../configgrpc-remove-deprecated-func2.yaml | 25 +++++++++++++++++++ config/configgrpc/configgrpc.go | 14 +++++------ config/configgrpc/configgrpc_test.go | 2 +- 4 files changed, 58 insertions(+), 8 deletions(-) create mode 100644 .chloggen/configgrpc-remove-deprecated-func.yaml create mode 100644 .chloggen/configgrpc-remove-deprecated-func2.yaml diff --git a/.chloggen/configgrpc-remove-deprecated-func.yaml b/.chloggen/configgrpc-remove-deprecated-func.yaml new file mode 100644 index 00000000000..6a6572d4de6 --- /dev/null +++ b/.chloggen/configgrpc-remove-deprecated-func.yaml @@ -0,0 +1,25 @@ +# Use this changelog template to create an entry for release notes. + +# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' +change_type: breaking + +# The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver) +component: configgrpc + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: Remove deprecated `ToServer` function. + +# One or more tracking issues or pull requests related to the change +issues: [9787] + +# (Optional) One or more lines of additional information to render under the primary note. +# These lines will be padded with 2 spaces and then inserted directly into the document. +# Use pipe (|) for multiline entries. +subtext: + +# Optional: The change log or logs in which this entry should be included. +# e.g. '[user]' or '[user, api]' +# Include 'user' if the change is relevant to end users. +# Include 'api' if there is a change to a library API. +# Default: '[user]' +change_logs: [api] diff --git a/.chloggen/configgrpc-remove-deprecated-func2.yaml b/.chloggen/configgrpc-remove-deprecated-func2.yaml new file mode 100644 index 00000000000..125b33da6fb --- /dev/null +++ b/.chloggen/configgrpc-remove-deprecated-func2.yaml @@ -0,0 +1,25 @@ +# Use this changelog template to create an entry for release notes. + +# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' +change_type: deprecation + +# The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver) +component: configgrpc + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: Deprecated `ToServerContext`, use `ToServer` instead. + +# One or more tracking issues or pull requests related to the change +issues: [9787] + +# (Optional) One or more lines of additional information to render under the primary note. +# These lines will be padded with 2 spaces and then inserted directly into the document. +# Use pipe (|) for multiline entries. +subtext: + +# Optional: The change log or logs in which this entry should be included. +# e.g. '[user]' or '[user, api]' +# Include 'user' if the change is relevant to end users. +# Include 'api' if there is a change to a library API. +# Default: '[user]' +change_logs: [api] diff --git a/config/configgrpc/configgrpc.go b/config/configgrpc/configgrpc.go index da592245d20..9460a008616 100644 --- a/config/configgrpc/configgrpc.go +++ b/config/configgrpc/configgrpc.go @@ -269,13 +269,7 @@ func validateBalancerName(balancerName string) bool { } // ToServer returns a grpc.Server for the configuration -// Deprecated: [0.96.0] Use ToServerContext instead. -func (gss *ServerConfig) ToServer(host component.Host, settings component.TelemetrySettings, extraOpts ...grpc.ServerOption) (*grpc.Server, error) { - return gss.ToServerContext(context.Background(), host, settings, extraOpts...) -} - -// ToServerContext returns a grpc.Server for the configuration -func (gss *ServerConfig) ToServerContext(_ context.Context, host component.Host, settings component.TelemetrySettings, extraOpts ...grpc.ServerOption) (*grpc.Server, error) { +func (gss *ServerConfig) ToServer(_ context.Context, host component.Host, settings component.TelemetrySettings, extraOpts ...grpc.ServerOption) (*grpc.Server, error) { opts, err := gss.toServerOption(host, settings) if err != nil { return nil, err @@ -284,6 +278,12 @@ func (gss *ServerConfig) ToServerContext(_ context.Context, host component.Host, return grpc.NewServer(opts...), nil } +// ToServerContext returns a grpc.Server for the configuration +// Deprecated: [v0.97.0] Use ToServer instead. +func (gss *ServerConfig) ToServerContext(ctx context.Context, host component.Host, settings component.TelemetrySettings, extraOpts ...grpc.ServerOption) (*grpc.Server, error) { + return gss.ToServer(ctx, host, settings, extraOpts...) +} + func (gss *ServerConfig) toServerOption(host component.Host, settings component.TelemetrySettings) ([]grpc.ServerOption, error) { switch gss.NetAddr.Transport { case confignet.TransportTypeTCP, confignet.TransportTypeTCP4, confignet.TransportTypeTCP6, confignet.TransportTypeUDP, confignet.TransportTypeUDP4, confignet.TransportTypeUDP6: diff --git a/config/configgrpc/configgrpc_test.go b/config/configgrpc/configgrpc_test.go index 20c853bbbc7..0e6a6efb0be 100644 --- a/config/configgrpc/configgrpc_test.go +++ b/config/configgrpc/configgrpc_test.go @@ -256,7 +256,7 @@ func TestGrpcServerAuthSettings_Deprecated(t *testing.T) { mockID: auth.NewServer(), }, } - srv, err := gss.ToServer(host, componenttest.NewNopTelemetrySettings()) + srv, err := gss.ToServer(context.Background(), host, componenttest.NewNopTelemetrySettings()) assert.NoError(t, err) assert.NotNil(t, srv) } From ef5d8f18cad912a69afbe9305150b9f09a1b94b9 Mon Sep 17 00:00:00 2001 From: KIMBOH LOVETTE <37558983+Kimbohlovette@users.noreply.github.com> Date: Thu, 21 Mar 2024 19:29:55 +0100 Subject: [PATCH 3/5] Nicer error message when passing an empty configuration file (#9762) This PR checks if `cfg.Validate()` error is `errMissingReceivers` error then returns a nicely formated error. --- otelcol/config.go | 10 ++++++++-- otelcol/config_test.go | 13 +++++++++++++ 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/otelcol/config.go b/otelcol/config.go index 9345405b4fd..7c27bd40201 100644 --- a/otelcol/config.go +++ b/otelcol/config.go @@ -12,8 +12,9 @@ import ( ) var ( - errMissingExporters = errors.New("no exporter configuration specified in config") - errMissingReceivers = errors.New("no receiver configuration specified in config") + errMissingExporters = errors.New("no exporter configuration specified in config") + errMissingReceivers = errors.New("no receiver configuration specified in config") + errEmptyConfigurationFile = errors.New("empty configuration file") ) // Config defines the configuration for the various elements of collector or agent. @@ -42,6 +43,11 @@ type Config struct { // invalid cases that we currently don't check for but which we may want to add in // the future (e.g. disallowing receiving and exporting on the same endpoint). func (cfg *Config) Validate() error { + // There must be at least one property set in the configuration file. + if len(cfg.Receivers) == 0 && len(cfg.Exporters) == 0 && len(cfg.Processors) == 0 && len(cfg.Connectors) == 0 && len(cfg.Extensions) == 0 { + return errEmptyConfigurationFile + } + // Currently, there is no default receiver enabled. // The configuration must specify at least one receiver to be valid. if len(cfg.Receivers) == 0 { diff --git a/otelcol/config_test.go b/otelcol/config_test.go index e482e6c8ae8..c1677c07220 100644 --- a/otelcol/config_test.go +++ b/otelcol/config_test.go @@ -54,6 +54,19 @@ func TestConfigValidate(t *testing.T) { }, expected: nil, }, + { + name: "empty configuration file", + cfgFn: func() *Config { + cfg := generateConfig() + cfg.Receivers = nil + cfg.Connectors = nil + cfg.Processors = nil + cfg.Exporters = nil + cfg.Extensions = nil + return cfg + }, + expected: errEmptyConfigurationFile, + }, { name: "missing-exporters", cfgFn: func() *Config { From 05867e63ac871040b790d91ffb85efdc4148b45f Mon Sep 17 00:00:00 2001 From: Joshua Jones Date: Thu, 21 Mar 2024 14:33:02 -0400 Subject: [PATCH 4/5] [otlphttpexporter] return nil from partial success handler when HTTP response body is empty (#9667) **Description:** Fixing a bug - When exporting using the otlphttpexporter, after receiving a successful HTTP response, when the response body's content length is 0 and the content type is specified as either "application/json" or "application/x-protobuf", an attempt will be made to unmarshal a nil value within any of the partial success response handler functions. This results in an error, and a potential resend of the original export request. To fix this scenario, a check was added to the `tracesPartialSuccessHandler`, `metricsPartialSuccessHandler`, and `logsPartialSuccessHandler` functions for a `nil` value in the `protoBytes` argument. When `nil`, the function will return with a `nil` value, indicating the absence of any error. **Link to tracking Issue:** #9666 --- .chloggen/fix_empty_response.yaml | 25 ++ exporter/otlphttpexporter/otlp.go | 9 + exporter/otlphttpexporter/otlp_test.go | 324 ++++++++++++++++++++----- 3 files changed, 298 insertions(+), 60 deletions(-) create mode 100644 .chloggen/fix_empty_response.yaml diff --git a/.chloggen/fix_empty_response.yaml b/.chloggen/fix_empty_response.yaml new file mode 100644 index 00000000000..66f1338c7ee --- /dev/null +++ b/.chloggen/fix_empty_response.yaml @@ -0,0 +1,25 @@ +# Use this changelog template to create an entry for release notes. + +# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' +change_type: bug_fix + +# The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver) +component: otlphttpexporter + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: prevent error on empty response body when content type is application/json + +# One or more tracking issues or pull requests related to the change +issues: [9666] + +# (Optional) One or more lines of additional information to render under the primary note. +# These lines will be padded with 2 spaces and then inserted directly into the document. +# Use pipe (|) for multiline entries. +subtext: + +# Optional: The change log or logs in which this entry should be included. +# e.g. '[user]' or '[user, api]' +# Include 'user' if the change is relevant to end users. +# Include 'api' if there is a change to a library API. +# Default: '[user]' +change_logs: [user] diff --git a/exporter/otlphttpexporter/otlp.go b/exporter/otlphttpexporter/otlp.go index 6ebc50e2cc0..8a8d428121b 100644 --- a/exporter/otlphttpexporter/otlp.go +++ b/exporter/otlphttpexporter/otlp.go @@ -301,6 +301,9 @@ func handlePartialSuccessResponse(resp *http.Response, partialSuccessHandler par type partialSuccessHandler func(bytes []byte, contentType string) error func (e *baseExporter) tracesPartialSuccessHandler(protoBytes []byte, contentType string) error { + if protoBytes == nil { + return nil + } exportResponse := ptraceotlp.NewExportResponse() switch contentType { case protobufContentType: @@ -328,6 +331,9 @@ func (e *baseExporter) tracesPartialSuccessHandler(protoBytes []byte, contentTyp } func (e *baseExporter) metricsPartialSuccessHandler(protoBytes []byte, contentType string) error { + if protoBytes == nil { + return nil + } exportResponse := pmetricotlp.NewExportResponse() switch contentType { case protobufContentType: @@ -355,6 +361,9 @@ func (e *baseExporter) metricsPartialSuccessHandler(protoBytes []byte, contentTy } func (e *baseExporter) logsPartialSuccessHandler(protoBytes []byte, contentType string) error { + if protoBytes == nil { + return nil + } exportResponse := plogotlp.NewExportResponse() switch contentType { case protobufContentType: diff --git a/exporter/otlphttpexporter/otlp_test.go b/exporter/otlphttpexporter/otlp_test.go index 0116803d816..470a4d872e2 100644 --- a/exporter/otlphttpexporter/otlp_test.go +++ b/exporter/otlphttpexporter/otlp_test.go @@ -37,6 +37,41 @@ import ( "go.opentelemetry.io/collector/pdata/ptrace/ptraceotlp" ) +const tracesTelemetryType = "traces" +const metricsTelemetryType = "metrics" +const logsTelemetryType = "logs" + +type responseSerializer interface { + MarshalJSON() ([]byte, error) + MarshalProto() ([]byte, error) +} + +type responseSerializerProvider = func() responseSerializer + +func provideTracesResponseSerializer() responseSerializer { + response := ptraceotlp.NewExportResponse() + partial := response.PartialSuccess() + partial.SetErrorMessage("hello") + partial.SetRejectedSpans(1) + return response +} + +func provideMetricsResponseSerializer() responseSerializer { + response := pmetricotlp.NewExportResponse() + partial := response.PartialSuccess() + partial.SetErrorMessage("hello") + partial.SetRejectedDataPoints(1) + return response +} + +func provideLogsResponseSerializer() responseSerializer { + response := plogotlp.NewExportResponse() + partial := response.PartialSuccess() + partial.SetErrorMessage("hello") + partial.SetRejectedLogRecords(1) + return response +} + func TestErrorResponses(t *testing.T) { errMsgPrefix := func(srv *httptest.Server) string { return fmt.Sprintf("error exporting items, request to %s/v1/traces responded with HTTP Status Code ", srv.URL) @@ -467,22 +502,66 @@ func TestPartialResponse_missingHeaderButHasBody(t *testing.T) { exp, err := newExporter(cfg, set) require.NoError(t, err) - response := ptraceotlp.NewExportResponse() - partial := response.PartialSuccess() - partial.SetErrorMessage("hello") - partial.SetRejectedSpans(1) - data, err := response.MarshalProto() - require.NoError(t, err) - resp := &http.Response{ - // `-1` indicates a missing Content-Length header in the Go http standard library - ContentLength: -1, - Body: io.NopCloser(bytes.NewReader(data)), - Header: map[string][]string{ - "Content-Type": {"application/x-protobuf"}, + contentTypes := []struct { + contentType string + }{ + {contentType: protobufContentType}, + {contentType: jsonContentType}, + } + + telemetryTypes := []struct { + telemetryType string + handler partialSuccessHandler + serializer responseSerializerProvider + }{ + { + telemetryType: tracesTelemetryType, + handler: exp.tracesPartialSuccessHandler, + serializer: provideTracesResponseSerializer, + }, + { + telemetryType: metricsTelemetryType, + handler: exp.metricsPartialSuccessHandler, + serializer: provideMetricsResponseSerializer, + }, + { + telemetryType: logsTelemetryType, + handler: exp.logsPartialSuccessHandler, + serializer: provideLogsResponseSerializer, }, } - err = handlePartialSuccessResponse(resp, exp.tracesPartialSuccessHandler) - assert.NoError(t, err) + + for _, ct := range contentTypes { + for _, tt := range telemetryTypes { + t.Run(tt.telemetryType+" "+ct.contentType, func(t *testing.T) { + serializer := tt.serializer() + + var data []byte + var err error + + switch ct.contentType { + case jsonContentType: + data, err = serializer.MarshalJSON() + case protobufContentType: + data, err = serializer.MarshalProto() + default: + require.Fail(t, "unsupported content type: %s", ct.contentType) + } + require.NoError(t, err) + + resp := &http.Response{ + // `-1` indicates a missing Content-Length header in the Go http standard library + ContentLength: -1, + Body: io.NopCloser(bytes.NewReader(data)), + Header: map[string][]string{ + "Content-Type": {ct.contentType}, + }, + } + err = handlePartialSuccessResponse(resp, tt.handler) + assert.NoError(t, err) + }) + } + } } func TestPartialResponse_missingHeaderAndBody(t *testing.T) { @@ -491,16 +570,47 @@ func TestPartialResponse_missingHeaderAndBody(t *testing.T) { exp, err := newExporter(cfg, set) require.NoError(t, err) - resp := &http.Response{ - // `-1` indicates a missing Content-Length header in the Go http standard library - ContentLength: -1, - Body: io.NopCloser(bytes.NewReader([]byte{})), - Header: map[string][]string{ - "Content-Type": {"application/x-protobuf"}, + contentTypes := []struct { + contentType string + }{ + {contentType: protobufContentType}, + {contentType: jsonContentType}, + } + + telemetryTypes := []struct { + telemetryType string + handler partialSuccessHandler + }{ + { + telemetryType: tracesTelemetryType, + handler: exp.tracesPartialSuccessHandler, + }, + { + telemetryType: metricsTelemetryType, + handler: exp.metricsPartialSuccessHandler, + }, + { + telemetryType: logsTelemetryType, + handler: exp.logsPartialSuccessHandler, }, } - err = handlePartialSuccessResponse(resp, exp.tracesPartialSuccessHandler) - assert.Nil(t, err) + + for _, ct := range contentTypes { + for _, tt := range telemetryTypes { + t.Run(tt.telemetryType+" "+ct.contentType, func(t *testing.T) { + resp := &http.Response{ + // `-1` indicates a missing Content-Length header in the Go http standard library + ContentLength: -1, + Body: io.NopCloser(bytes.NewReader([]byte{})), + Header: map[string][]string{ + "Content-Type": {ct.contentType}, + }, + } + err = handlePartialSuccessResponse(resp, tt.handler) + assert.Nil(t, err) + }) + } + } } func TestPartialResponse_nonErrUnexpectedEOFError(t *testing.T) { @@ -524,53 +634,147 @@ func TestPartialSuccess_shortContentLengthHeader(t *testing.T) { exp, err := newExporter(cfg, set) require.NoError(t, err) - response := ptraceotlp.NewExportResponse() - partial := response.PartialSuccess() - partial.SetErrorMessage("hello") - partial.SetRejectedSpans(1) - data, err := response.MarshalProto() - require.NoError(t, err) - resp := &http.Response{ - ContentLength: 3, - Body: io.NopCloser(bytes.NewReader(data)), - Header: map[string][]string{ - "Content-Type": {"application/x-protobuf"}, + contentTypes := []struct { + contentType string + }{ + {contentType: protobufContentType}, + {contentType: jsonContentType}, + } + + telemetryTypes := []struct { + telemetryType string + handler partialSuccessHandler + serializer responseSerializerProvider + }{ + { + telemetryType: tracesTelemetryType, + handler: exp.tracesPartialSuccessHandler, + serializer: provideTracesResponseSerializer, + }, + { + telemetryType: metricsTelemetryType, + handler: exp.metricsPartialSuccessHandler, + serializer: provideMetricsResponseSerializer, + }, + { + telemetryType: logsTelemetryType, + handler: exp.logsPartialSuccessHandler, + serializer: provideLogsResponseSerializer, }, } - // For short content-length, a real error happens. - err = handlePartialSuccessResponse(resp, exp.tracesPartialSuccessHandler) - assert.Error(t, err) + + for _, ct := range contentTypes { + for _, tt := range telemetryTypes { + t.Run(tt.telemetryType+" "+ct.contentType, func(t *testing.T) { + serializer := tt.serializer() + + var data []byte + var err error + + switch ct.contentType { + case jsonContentType: + data, err = serializer.MarshalJSON() + case protobufContentType: + data, err = serializer.MarshalProto() + default: + require.Fail(t, "unsupported content type: %s", ct.contentType) + } + require.NoError(t, err) + + resp := &http.Response{ + ContentLength: 3, + Body: io.NopCloser(bytes.NewReader(data)), + Header: map[string][]string{ + "Content-Type": {ct.contentType}, + }, + } + // For short content-length, a real error happens. + err = handlePartialSuccessResponse(resp, tt.handler) + assert.Error(t, err) + }) + } + } } func TestPartialSuccess_longContentLengthHeader(t *testing.T) { - cfg := createDefaultConfig() - set := exportertest.NewNopCreateSettings() + contentTypes := []struct { + contentType string + }{ + {contentType: protobufContentType}, + {contentType: jsonContentType}, + } - logger, observed := observer.New(zap.DebugLevel) - set.TelemetrySettings.Logger = zap.New(logger) + telemetryTypes := []struct { + telemetryType string + serializer responseSerializerProvider + }{ + { + telemetryType: tracesTelemetryType, + serializer: provideTracesResponseSerializer, + }, + { + telemetryType: metricsTelemetryType, + serializer: provideMetricsResponseSerializer, + }, + { + telemetryType: logsTelemetryType, + serializer: provideLogsResponseSerializer, + }, + } - exp, err := newExporter(cfg, set) - require.NoError(t, err) + for _, ct := range contentTypes { + for _, tt := range telemetryTypes { + t.Run(tt.telemetryType+" "+ct.contentType, func(t *testing.T) { + cfg := createDefaultConfig() + set := exportertest.NewNopCreateSettings() + logger, observed := observer.New(zap.DebugLevel) + set.TelemetrySettings.Logger = zap.New(logger) + exp, err := newExporter(cfg, set) + require.NoError(t, err) - response := ptraceotlp.NewExportResponse() - partial := response.PartialSuccess() - partial.SetErrorMessage("hello") - partial.SetRejectedSpans(1) - data, err := response.MarshalProto() - require.NoError(t, err) - resp := &http.Response{ - ContentLength: 4096, - Body: io.NopCloser(bytes.NewReader(data)), - Header: map[string][]string{ - "Content-Type": {"application/x-protobuf"}, - }, + serializer := tt.serializer() + + var handler partialSuccessHandler + + switch tt.telemetryType { + case tracesTelemetryType: + handler = exp.tracesPartialSuccessHandler + case metricsTelemetryType: + handler = exp.metricsPartialSuccessHandler + case logsTelemetryType: + handler = exp.logsPartialSuccessHandler + default: + require.Fail(t, "unsupported telemetry type: %s", ct.contentType) + } + + var data []byte + + switch ct.contentType { + case jsonContentType: + data, err = serializer.MarshalJSON() + case protobufContentType: + data, err = serializer.MarshalProto() + default: + require.Fail(t, "unsupported content type: %s", ct.contentType) + } + require.NoError(t, err) + + resp := &http.Response{ + ContentLength: 4096, + Body: io.NopCloser(bytes.NewReader(data)), + Header: map[string][]string{ + "Content-Type": {ct.contentType}, + }, + } + // No real error happens for long content length, so the partial + // success is handled as success with a warning. + err = handlePartialSuccessResponse(resp, handler) + assert.NoError(t, err) + assert.Len(t, observed.FilterLevelExact(zap.WarnLevel).All(), 1) + assert.Contains(t, observed.FilterLevelExact(zap.WarnLevel).All()[0].Message, "Partial success") + }) + } } - // No real error happens for long content length, so the partial - // success is handled as success with a warning. - err = handlePartialSuccessResponse(resp, exp.tracesPartialSuccessHandler) - assert.NoError(t, err) - assert.Len(t, observed.FilterLevelExact(zap.WarnLevel).All(), 1) - assert.Contains(t, observed.FilterLevelExact(zap.WarnLevel).All()[0].Message, "Partial success") } func TestPartialSuccessInvalidResponseBody(t *testing.T) { From 2037527386ab7e4845d8b7fc10488b0ea5996a38 Mon Sep 17 00:00:00 2001 From: Antoine Toulme Date: Thu, 21 Mar 2024 11:34:17 -0700 Subject: [PATCH 5/5] [chore] arm64 build (#9584) This PR adds a linux/arm64 build to the build of the collector, so it may support the goal of #9731 --- .github/workflows/build-and-test.yml | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml index bedc15548a8..4f617c9ca61 100644 --- a/.github/workflows/build-and-test.yml +++ b/.github/workflows/build-and-test.yml @@ -137,10 +137,18 @@ jobs: unittest-matrix: strategy: matrix: + runner: [ubuntu-latest, actuated-arm64-4cpu-4gb] go-version: ["~1.22", "~1.21.8"] # 1.20 needs quotes otherwise it's interpreted as 1.2 - runs-on: ubuntu-latest + runs-on: ${{ matrix.runner }} needs: [setup-environment] steps: + - name: Set up arkade + uses: alexellis/setup-arkade@v3 + - name: Install vmmeter + run: | + sudo -E arkade oci install ghcr.io/openfaasltd/vmmeter:latest --path /usr/local/bin/ + - name: Run vmmeter + uses: self-actuated/vmmeter-action@v1 - name: Checkout Repo uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1 - name: Setup Go @@ -155,14 +163,15 @@ jobs: path: | ~/go/bin ~/go/pkg/mod - key: go-cache-${{ runner.os }}-${{ hashFiles('**/go.sum') }} + key: go-cache-${{ runner.os }}-${{ matrix.runner }}-${{ hashFiles('**/go.sum') }} - name: Cache Build uses: actions/cache@0c45773b623bea8c8e75f6c82b208c3cf94ea4f9 # v4.0.2 with: path: ~/.cache/go-build - key: unittest-${{ runner.os }}-go-build-${{ matrix.go-version }}-${{ hashFiles('**/go.sum') }} + key: unittest-${{ runner.os }}-${{ matrix.runner }}-go-build-${{ matrix.go-version }}-${{ hashFiles('**/go.sum') }} - name: Run Unit Tests - run: make gotest + run: | + make -j4 gotest unittest: if: always() runs-on: ubuntu-latest