Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[exporter/kafka] move kafka configures authentication to internal pkg #27289

Merged
merged 1 commit into from
Oct 9, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions .chloggen/chore-kafka-configures-authentication.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: 'enhancement'

# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver)
component: kafkaexporter

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: Move kafka configures authentication to internal pkg

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

# (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: []
1 change: 1 addition & 0 deletions .github/CODEOWNERS
Validating CODEOWNERS rules …
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ internal/docker/ @open-te
internal/filter/ @open-telemetry/collector-contrib-approvers @open-telemetry/collector-approvers
internal/k8sconfig/ @open-telemetry/collector-contrib-approvers @dmitryax
internal/k8stest/ @open-telemetry/collector-contrib-approvers @crobert-1
internal/kafka/ @open-telemetry/collector-contrib-approvers @pavolloffay @MovieStoreGuy
internal/kubelet/ @open-telemetry/collector-contrib-approvers @dmitryax
internal/metadataproviders/ @open-telemetry/collector-contrib-approvers @Aneurysm9 @dashpole
internal/sharedcomponent/ @open-telemetry/collector-contrib-approvers @open-telemetry/collector-approvers
Expand Down
1 change: 1 addition & 0 deletions .github/ISSUE_TEMPLATE/bug_report.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ body:
- internal/filter
- internal/k8sconfig
- internal/k8stest
- internal/kafka
- internal/kubelet
- internal/metadataproviders
- internal/sharedcomponent
Expand Down
1 change: 1 addition & 0 deletions .github/ISSUE_TEMPLATE/feature_request.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ body:
- internal/filter
- internal/k8sconfig
- internal/k8stest
- internal/kafka
- internal/kubelet
- internal/metadataproviders
- internal/sharedcomponent
Expand Down
1 change: 1 addition & 0 deletions .github/ISSUE_TEMPLATE/other.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ body:
- internal/filter
- internal/k8sconfig
- internal/k8stest
- internal/kafka
- internal/kubelet
- internal/metadataproviders
- internal/sharedcomponent
Expand Down
10 changes: 5 additions & 5 deletions .github/dependabot.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,6 @@ updates:
schedule:
interval: "weekly"
day: "wednesday"
- package-ecosystem: "gomod"
directory: "/cmd/opampsupervisor"
schedule:
interval: "weekly"
day: "wednesday"
- package-ecosystem: "gomod"
directory: "/cmd/otelcontribcol"
schedule:
Expand Down Expand Up @@ -417,6 +412,11 @@ updates:
schedule:
interval: "weekly"
day: "wednesday"
- package-ecosystem: "gomod"
directory: "/internal/kafka"
schedule:
interval: "weekly"
day: "wednesday"
- package-ecosystem: "gomod"
directory: "/internal/kubelet"
schedule:
Expand Down
3 changes: 3 additions & 0 deletions cmd/configschema/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -493,6 +493,7 @@ require (
github.com/open-telemetry/opentelemetry-collector-contrib/internal/aws/xray v0.86.1-0.20231006161201-d364ad61c4d7 // indirect
github.com/open-telemetry/opentelemetry-collector-contrib/internal/common v0.86.1-0.20231006161201-d364ad61c4d7 // indirect
github.com/open-telemetry/opentelemetry-collector-contrib/internal/datadog v0.86.1-0.20231006161201-d364ad61c4d7 // indirect
github.com/open-telemetry/opentelemetry-collector-contrib/internal/kafka v0.86.1-0.20231006161201-d364ad61c4d7 // indirect
github.com/open-telemetry/opentelemetry-collector-contrib/internal/sharedcomponent v0.86.1-0.20231006161201-d364ad61c4d7 // indirect
github.com/open-telemetry/opentelemetry-collector-contrib/pkg/experimentalmetricmetadata v0.86.1-0.20231006161201-d364ad61c4d7 // indirect
github.com/open-telemetry/opentelemetry-collector-contrib/pkg/winperfcounters v0.86.1-0.20231006161201-d364ad61c4d7 // indirect
Expand Down Expand Up @@ -724,6 +725,8 @@ replace github.com/open-telemetry/opentelemetry-collector-contrib/internal/docke

replace github.com/open-telemetry/opentelemetry-collector-contrib/internal/k8sconfig => ../../internal/k8sconfig

replace github.com/open-telemetry/opentelemetry-collector-contrib/internal/kafka => ../../internal/kafka

replace github.com/open-telemetry/opentelemetry-collector-contrib/internal/kubelet => ../../internal/kubelet

replace github.com/open-telemetry/opentelemetry-collector-contrib/internal/metadataproviders => ../../internal/metadataproviders
Expand Down
1 change: 1 addition & 0 deletions cmd/otelcontribcol/builder-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -314,6 +314,7 @@ replaces:
- github.com/open-telemetry/opentelemetry-collector-contrib/exporter/azuredataexplorerexporter => ../../exporter/azuredataexplorerexporter
- github.com/open-telemetry/opentelemetry-collector-contrib/internal/k8sconfig => ../../internal/k8sconfig
- github.com/open-telemetry/opentelemetry-collector-contrib/internal/k8stest => ../../internal/k8stest
- github.com/open-telemetry/opentelemetry-collector-contrib/internal/kafka => ../../internal/kafka
- github.com/open-telemetry/opentelemetry-collector-contrib/receiver/carbonreceiver => ../../receiver/carbonreceiver
- github.com/open-telemetry/opentelemetry-collector-contrib/exporter/splunkhecexporter => ../../exporter/splunkhecexporter
- github.com/open-telemetry/opentelemetry-collector-contrib/exporter/prometheusexporter => ../../exporter/prometheusexporter
Expand Down
3 changes: 3 additions & 0 deletions cmd/otelcontribcol/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -522,6 +522,7 @@ require (
github.com/open-telemetry/opentelemetry-collector-contrib/internal/docker v0.86.1-0.20231006161201-d364ad61c4d7 // indirect
github.com/open-telemetry/opentelemetry-collector-contrib/internal/filter v0.86.1-0.20231006161201-d364ad61c4d7 // indirect
github.com/open-telemetry/opentelemetry-collector-contrib/internal/k8sconfig v0.86.1-0.20231006161201-d364ad61c4d7 // indirect
github.com/open-telemetry/opentelemetry-collector-contrib/internal/kafka v0.86.1-0.20231006161201-d364ad61c4d7 // indirect
github.com/open-telemetry/opentelemetry-collector-contrib/internal/kubelet v0.86.1-0.20231006161201-d364ad61c4d7 // indirect
github.com/open-telemetry/opentelemetry-collector-contrib/internal/metadataproviders v0.86.1-0.20231006161201-d364ad61c4d7 // indirect
github.com/open-telemetry/opentelemetry-collector-contrib/internal/sharedcomponent v0.86.1-0.20231006161201-d364ad61c4d7 // indirect
Expand Down Expand Up @@ -930,6 +931,8 @@ replace github.com/open-telemetry/opentelemetry-collector-contrib/internal/k8sco

replace github.com/open-telemetry/opentelemetry-collector-contrib/internal/k8stest => ../../internal/k8stest

replace github.com/open-telemetry/opentelemetry-collector-contrib/internal/kafka => ../../internal/kafka

replace github.com/open-telemetry/opentelemetry-collector-contrib/receiver/carbonreceiver => ../../receiver/carbonreceiver

replace github.com/open-telemetry/opentelemetry-collector-contrib/exporter/splunkhecexporter => ../../exporter/splunkhecexporter
Expand Down
6 changes: 4 additions & 2 deletions exporter/kafkaexporter/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import (
"github.com/IBM/sarama"
"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/exporter/exporterhelper"

"github.com/open-telemetry/opentelemetry-collector-contrib/internal/kafka"
)

// Config defines configuration for Kafka exporter.
Expand All @@ -36,7 +38,7 @@ type Config struct {
Producer Producer `mapstructure:"producer"`

// Authentication defines used authentication mechanism.
Authentication Authentication `mapstructure:"auth"`
Authentication kafka.Authentication `mapstructure:"auth"`
}

// Metadata defines configuration for retrieving metadata from the broker.
Expand Down Expand Up @@ -103,7 +105,7 @@ func (cfg *Config) Validate() error {
return validateSASLConfig(cfg.Authentication.SASL)
}

func validateSASLConfig(c *SASLConfig) error {
func validateSASLConfig(c *kafka.SASLConfig) error {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Speaking of validation, how about defining a common validation function for kafka.Authentication in internal/kafka package, which can be reused by kafkametricsreceiver, kafkareceiver and kafkaexporter.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It sounds great, it seems duplicate logic
in between validateSASLConfig and configureSASL, maybe we can defining a common validation function as you said :), and we can make a separate issue to follow up

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you help create an issue for this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure! I am glad to do that, i will make it a few days later since without bringing laptop currently

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you help create an issue for this?

Sorry for delay, #27486

if c == nil {
return nil
}
Expand Down
31 changes: 16 additions & 15 deletions exporter/kafkaexporter/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"go.opentelemetry.io/collector/exporter/exporterhelper"

"github.com/open-telemetry/opentelemetry-collector-contrib/exporter/kafkaexporter/internal/metadata"
"github.com/open-telemetry/opentelemetry-collector-contrib/internal/kafka"
)

func TestLoadConfig(t *testing.T) {
Expand Down Expand Up @@ -56,8 +57,8 @@ func TestLoadConfig(t *testing.T) {
Topic: "spans",
Encoding: "otlp_proto",
Brokers: []string{"foo:123", "bar:456"},
Authentication: Authentication{
PlainText: &PlainTextConfig{
Authentication: kafka.Authentication{
PlainText: &kafka.PlainTextConfig{
Username: "jdoe",
Password: "pass",
},
Expand All @@ -79,8 +80,8 @@ func TestLoadConfig(t *testing.T) {
{
id: component.NewIDWithName(metadata.Type, ""),
option: func(conf *Config) {
conf.Authentication = Authentication{
SASL: &SASLConfig{
conf.Authentication = kafka.Authentication{
SASL: &kafka.SASLConfig{
Username: "jdoe",
Password: "pass",
Mechanism: "PLAIN",
Expand Down Expand Up @@ -108,12 +109,12 @@ func TestLoadConfig(t *testing.T) {
Topic: "spans",
Encoding: "otlp_proto",
Brokers: []string{"foo:123", "bar:456"},
Authentication: Authentication{
PlainText: &PlainTextConfig{
Authentication: kafka.Authentication{
PlainText: &kafka.PlainTextConfig{
Username: "jdoe",
Password: "pass",
},
SASL: &SASLConfig{
SASL: &kafka.SASLConfig{
Username: "jdoe",
Password: "pass",
Mechanism: "PLAIN",
Expand Down Expand Up @@ -166,8 +167,8 @@ func TestValidate_sasl_username(t *testing.T) {
Producer: Producer{
Compression: "none",
},
Authentication: Authentication{
SASL: &SASLConfig{
Authentication: kafka.Authentication{
SASL: &kafka.SASLConfig{
Username: "",
Password: "pass",
Mechanism: "PLAIN",
Expand All @@ -184,8 +185,8 @@ func TestValidate_sasl_password(t *testing.T) {
Producer: Producer{
Compression: "none",
},
Authentication: Authentication{
SASL: &SASLConfig{
Authentication: kafka.Authentication{
SASL: &kafka.SASLConfig{
Username: "jdoe",
Password: "",
Mechanism: "PLAIN",
Expand All @@ -202,8 +203,8 @@ func TestValidate_sasl_mechanism(t *testing.T) {
Producer: Producer{
Compression: "none",
},
Authentication: Authentication{
SASL: &SASLConfig{
Authentication: kafka.Authentication{
SASL: &kafka.SASLConfig{
Username: "jdoe",
Password: "pass",
Mechanism: "FAKE",
Expand All @@ -220,8 +221,8 @@ func TestValidate_sasl_version(t *testing.T) {
Producer: Producer{
Compression: "none",
},
Authentication: Authentication{
SASL: &SASLConfig{
Authentication: kafka.Authentication{
SASL: &kafka.SASLConfig{
Username: "jdoe",
Password: "pass",
Mechanism: "PLAIN",
Expand Down
11 changes: 7 additions & 4 deletions exporter/kafkaexporter/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,15 @@ go 1.20

require (
github.com/IBM/sarama v1.41.2
github.com/aws/aws-sdk-go v1.45.20
github.com/cenkalti/backoff/v4 v4.2.1
github.com/gogo/protobuf v1.3.2
github.com/jaegertracing/jaeger v1.48.0
github.com/open-telemetry/opentelemetry-collector-contrib/internal/coreinternal v0.86.1-0.20231006161201-d364ad61c4d7
github.com/open-telemetry/opentelemetry-collector-contrib/internal/kafka v0.86.1-0.20231006161201-d364ad61c4d7
github.com/open-telemetry/opentelemetry-collector-contrib/pkg/translator/jaeger v0.86.1-0.20231006161201-d364ad61c4d7
github.com/open-telemetry/opentelemetry-collector-contrib/pkg/translator/zipkin v0.86.1-0.20231006161201-d364ad61c4d7
github.com/openzipkin/zipkin-go v0.4.2
github.com/stretchr/testify v1.8.4
github.com/xdg-go/scram v1.1.2
go.opentelemetry.io/collector/component v0.86.1-0.20231006161201-d364ad61c4d7
go.opentelemetry.io/collector/config/configtls v0.86.1-0.20231006161201-d364ad61c4d7
go.opentelemetry.io/collector/confmap v0.86.1-0.20231006161201-d364ad61c4d7
Expand All @@ -27,7 +26,8 @@ require (

require (
github.com/apache/thrift v0.19.0 // indirect
github.com/davecgh/go-spew v1.1.1 // indirect
github.com/aws/aws-sdk-go v1.45.20 // indirect
github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect
github.com/eapache/go-resiliency v1.4.0 // indirect
github.com/eapache/go-xerial-snappy v0.0.0-20230731223053-c322873962e3 // indirect
github.com/eapache/queue v1.1.0 // indirect
Expand All @@ -54,11 +54,12 @@ require (
github.com/modern-go/reflect2 v1.0.2 // indirect
github.com/opentracing/opentracing-go v1.2.0 // indirect
github.com/pierrec/lz4/v4 v4.1.18 // indirect
github.com/pmezard/go-difflib v1.0.0 // indirect
github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect
github.com/rcrowley/go-metrics v0.0.0-20201227073835-cf1acfcdf475 // indirect
github.com/uber/jaeger-client-go v2.30.0+incompatible // indirect
github.com/uber/jaeger-lib v2.4.1+incompatible // indirect
github.com/xdg-go/pbkdf2 v1.0.0 // indirect
github.com/xdg-go/scram v1.1.2 // indirect
github.com/xdg-go/stringprep v1.0.4 // indirect
go.opencensus.io v0.24.0 // indirect
go.opentelemetry.io/collector v0.86.1-0.20231006161201-d364ad61c4d7 // indirect
Expand All @@ -82,6 +83,8 @@ require (

replace github.com/open-telemetry/opentelemetry-collector-contrib/internal/coreinternal => ../../internal/coreinternal

replace github.com/open-telemetry/opentelemetry-collector-contrib/internal/kafka => ../../internal/kafka

replace github.com/open-telemetry/opentelemetry-collector-contrib/pkg/translator/jaeger => ../../pkg/translator/jaeger

retract (
Expand Down
6 changes: 4 additions & 2 deletions exporter/kafkaexporter/go.sum

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

4 changes: 3 additions & 1 deletion exporter/kafkaexporter/kafka_exporter.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ import (
"go.opentelemetry.io/collector/pdata/pmetric"
"go.opentelemetry.io/collector/pdata/ptrace"
"go.uber.org/zap"

"github.com/open-telemetry/opentelemetry-collector-contrib/internal/kafka"
)

var errUnrecognizedEncoding = fmt.Errorf("unrecognized encoding")
Expand Down Expand Up @@ -140,7 +142,7 @@ func newSaramaProducer(config Config) (sarama.SyncProducer, error) {
c.Version = version
}

if err := ConfigureAuthentication(config.Authentication, c); err != nil {
if err := kafka.ConfigureAuthentication(config.Authentication, c); err != nil {
return nil, err
}

Expand Down
3 changes: 2 additions & 1 deletion exporter/kafkaexporter/kafka_exporter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"go.uber.org/zap"

"github.com/open-telemetry/opentelemetry-collector-contrib/internal/coreinternal/testdata"
"github.com/open-telemetry/opentelemetry-collector-contrib/internal/kafka"
)

func TestNewExporter_err_version(t *testing.T) {
Expand Down Expand Up @@ -81,7 +82,7 @@ func TestNewLogsExporter_err_traces_encoding(t *testing.T) {
func TestNewExporter_err_auth_type(t *testing.T) {
c := Config{
ProtocolVersion: "2.0.0",
Authentication: Authentication{
Authentication: kafka.Authentication{
TLS: &configtls.TLSClientSetting{
TLSSetting: configtls.TLSSetting{
CAFile: "/doesnotexist",
Expand Down
3 changes: 3 additions & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -500,6 +500,7 @@ require (
github.com/open-telemetry/opentelemetry-collector-contrib/internal/docker v0.86.1-0.20231006161201-d364ad61c4d7 // indirect
github.com/open-telemetry/opentelemetry-collector-contrib/internal/filter v0.86.1-0.20231006161201-d364ad61c4d7 // indirect
github.com/open-telemetry/opentelemetry-collector-contrib/internal/k8sconfig v0.86.1-0.20231006161201-d364ad61c4d7 // indirect
github.com/open-telemetry/opentelemetry-collector-contrib/internal/kafka v0.86.1-0.20231006161201-d364ad61c4d7 // indirect
github.com/open-telemetry/opentelemetry-collector-contrib/internal/kubelet v0.86.1-0.20231006161201-d364ad61c4d7 // indirect
github.com/open-telemetry/opentelemetry-collector-contrib/internal/metadataproviders v0.86.1-0.20231006161201-d364ad61c4d7 // indirect
github.com/open-telemetry/opentelemetry-collector-contrib/internal/sharedcomponent v0.86.1-0.20231006161201-d364ad61c4d7 // indirect
Expand Down Expand Up @@ -725,6 +726,8 @@ replace github.com/open-telemetry/opentelemetry-collector-contrib/internal/k8sco

replace github.com/open-telemetry/opentelemetry-collector-contrib/internal/k8stest => ./internal/k8stest

replace github.com/open-telemetry/opentelemetry-collector-contrib/internal/kafka => ./internal/kafka

replace github.com/open-telemetry/opentelemetry-collector-contrib/internal/kubelet => ./internal/kubelet

replace github.com/open-telemetry/opentelemetry-collector-contrib/internal/metadataproviders => ./internal/metadataproviders
Expand Down
1 change: 1 addition & 0 deletions internal/kafka/Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
include ../../Makefile.Common
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Copyright The OpenTelemetry Authors
// SPDX-License-Identifier: Apache-2.0

package kafkaexporter // import "github.com/open-telemetry/opentelemetry-collector-contrib/exporter/kafkaexporter"
package kafka // import "github.com/open-telemetry/opentelemetry-collector-contrib/internal/kafka"

import (
"crypto/sha256"
Expand All @@ -11,7 +11,7 @@ import (
"github.com/IBM/sarama"
"go.opentelemetry.io/collector/config/configtls"

"github.com/open-telemetry/opentelemetry-collector-contrib/exporter/kafkaexporter/internal/awsmsk"
"github.com/open-telemetry/opentelemetry-collector-contrib/internal/kafka/awsmsk"
)

// Authentication defines authentication.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Copyright The OpenTelemetry Authors
// SPDX-License-Identifier: Apache-2.0

package kafkaexporter
package kafka

import (
"testing"
Expand Down
Loading