Skip to content

Commit

Permalink
[encoding/zipkinencodingextension] add default case (#28689)
Browse files Browse the repository at this point in the history
**Description:** Fix bug when err is nil if an invalid version value is
supplied.

**Link to tracking Issue:** #28686

---------

Co-authored-by: Dmitrii Anoshin <anoshindx@gmail.com>
  • Loading branch information
VihasMakwana and dmitryax authored Nov 1, 2023
1 parent 1c63ac9 commit 7efdf16
Show file tree
Hide file tree
Showing 5 changed files with 69 additions and 2 deletions.
27 changes: 27 additions & 0 deletions .chloggen/bugfix-zipkin-encoding.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
# 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. filelogreceiver)
component: extension/zipkinencodingextension

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: Fix bug when err is nil if invalid protocol value is supplied.

# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists.
issues: [28686]

# (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:

# If your change doesn't affect end users or the exported elements of any package,
# you should instead start your pull request title with [chore] or use the "Skip Changelog" label.
# 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: []
6 changes: 4 additions & 2 deletions extension/encoding/zipkinencodingextension/extension.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ func newExtension(config *Config) (*zipkinExtension, error) {
unmarshaler: zipkinv2.NewProtobufTracesUnmarshaler(false, false),
}
default:
err = fmt.Errorf("unsupported version: %q and protocol: %q", version, protocol)
err = fmt.Errorf("protocol: %q, unsupported version: %q", protocol, version)
}
case zipkinJSONEncoding:
switch version {
Expand All @@ -77,8 +77,10 @@ func newExtension(config *Config) (*zipkinExtension, error) {
unmarshaler: zipkinv1.NewThriftTracesUnmarshaler(),
}
default:
err = fmt.Errorf("unsupported version: %q and protocol: %q", version, protocol)
err = fmt.Errorf("protocol: %q, unsupported version: %q", protocol, version)
}
default:
err = fmt.Errorf("unsupported protocol: %q", protocol)
}

return ex, err
Expand Down
30 changes: 30 additions & 0 deletions extension/encoding/zipkinencodingextension/extension_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
// Copyright The OpenTelemetry Authors
// SPDX-License-Identifier: Apache-2.0

package zipkinencodingextension // import "github.com/open-telemetry/opentelemetry-collector-contrib/extension/encoding/zipkinencodingextension"

import (
"context"
"testing"

"github.com/stretchr/testify/require"
"go.opentelemetry.io/collector/component/componenttest"
"go.opentelemetry.io/collector/extension/extensiontest"
)

func TestExtension_Start(t *testing.T) {
j := &zipkinExtension{
config: createDefaultConfig().(*Config),
}
err := j.Start(context.Background(), componenttest.NewNopHost())
require.NoError(t, err)
}

func TestExtension_Err(t *testing.T) {
factory := NewFactory()
cfg := createDefaultConfig().(*Config)
cfg.Protocol = "v3"
_, err := factory.CreateExtension(context.Background(), extensiontest.NewNopCreateSettings(), cfg)
require.NotNil(t, err)
require.ErrorContains(t, err, "unsupported protocol: \"v3\"")
}
4 changes: 4 additions & 0 deletions extension/encoding/zipkinencodingextension/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,15 @@ go 1.20
require (
github.com/open-telemetry/opentelemetry-collector-contrib/extension/encoding v0.0.0-00010101000000-000000000000
github.com/open-telemetry/opentelemetry-collector-contrib/pkg/translator/zipkin v0.88.0
github.com/stretchr/testify v1.8.4
go.opentelemetry.io/collector/component v0.88.1-0.20231026220224-6405e152a2d9
go.opentelemetry.io/collector/extension v0.88.1-0.20231026220224-6405e152a2d9
go.opentelemetry.io/collector/pdata v1.0.0-rcv0017.0.20231026220224-6405e152a2d9
)

require (
github.com/apache/thrift v0.19.0 // indirect
github.com/davecgh/go-spew v1.1.1 // indirect
github.com/gogo/protobuf v1.3.2 // indirect
github.com/golang/protobuf v1.5.3 // indirect
github.com/jaegertracing/jaeger v1.48.0 // indirect
Expand All @@ -27,6 +29,7 @@ require (
github.com/open-telemetry/opentelemetry-collector-contrib/internal/coreinternal v0.88.0 // indirect
github.com/opentracing/opentracing-go v1.2.0 // indirect
github.com/openzipkin/zipkin-go v0.4.2 // indirect
github.com/pmezard/go-difflib v1.0.0 // indirect
github.com/uber/jaeger-client-go v2.30.0+incompatible // indirect
github.com/uber/jaeger-lib v2.4.1+incompatible // indirect
go.opentelemetry.io/collector/config/configtelemetry v0.88.1-0.20231026220224-6405e152a2d9 // indirect
Expand All @@ -45,6 +48,7 @@ require (
google.golang.org/genproto/googleapis/rpc v0.0.0-20230822172742-b8732ec3820d // indirect
google.golang.org/grpc v1.59.0 // indirect
google.golang.org/protobuf v1.31.0 // indirect
gopkg.in/yaml.v3 v3.0.1 // indirect
)

replace github.com/open-telemetry/opentelemetry-collector-contrib/internal/coreinternal => ../../../internal/coreinternal
Expand Down
4 changes: 4 additions & 0 deletions extension/encoding/zipkinencodingextension/go.sum

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 7efdf16

Please sign in to comment.