-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Feature/kakfa exporter - kafka key by TraceID #25909
Changes from all commits
3c3b675
3f0ccbb
19b29d2
21421bc
371ae71
9bd2428
6d26b8b
175b24b
c1f045a
2f7c207
3b9388e
ae8fefe
9c7bb5a
f577e95
ca210ca
e09138e
6568027
709bc5a
cb25535
2789405
dde0fa3
495de39
62c50c6
39c3467
4134f9d
8550caf
e5c230e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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: add ability to publish kafka messages with key of TraceID - it will allow to partition the kafka Topic and consume it concurrently. | ||
|
||
# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists. | ||
issues: [12318] | ||
|
||
# (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: [user, api] |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -21,6 +21,7 @@ const ( | |
defaultMetricsTopic = "otlp_metrics" | ||
defaultLogsTopic = "otlp_logs" | ||
defaultEncoding = "otlp_proto" | ||
defaultKeyData = "none" | ||
defaultBroker = "localhost:9092" | ||
// default from sarama.NewConfig() | ||
defaultMetadataRetryMax = 3 | ||
|
@@ -45,7 +46,7 @@ type FactoryOption func(factory *kafkaExporterFactory) | |
func WithTracesMarshalers(tracesMarshalers ...TracesMarshaler) FactoryOption { | ||
return func(factory *kafkaExporterFactory) { | ||
for _, marshaler := range tracesMarshalers { | ||
factory.tracesMarshalers[marshaler.Encoding()] = marshaler | ||
factory.tracesMarshalers[KeyOfTracerMarshaller(marshaler)] = marshaler | ||
} | ||
} | ||
} | ||
|
@@ -96,6 +97,7 @@ func createDefaultConfig() component.Config { | |
// using an empty topic to track when it has not been set by user, default is based on traces or metrics. | ||
Topic: "", | ||
Encoding: defaultEncoding, | ||
KeyData: defaultKeyData, | ||
Metadata: Metadata{ | ||
Full: defaultMetadataFull, | ||
Retry: MetadataRetry{ | ||
|
@@ -118,6 +120,14 @@ type kafkaExporterFactory struct { | |
logsMarshalers map[string]LogsMarshaler | ||
} | ||
|
||
func KeyOfTracerMarshaller(marshaler TracesMarshaler) string { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This function doesn't it clear of the intent, it should also not be exported. Could I ask you to rename add some comments around it to help explain its usage. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the key used to be only the |
||
return KeyOfTracerMarshallerBy(marshaler.Encoding(), marshaler.KeyData()) | ||
} | ||
|
||
func KeyOfTracerMarshallerBy(encoding string, keyData string) string { | ||
return encoding + "#" + keyData | ||
} | ||
|
||
func (f *kafkaExporterFactory) createTracesExporter( | ||
ctx context.Context, | ||
set exporter.CreateSettings, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,6 +18,7 @@ import ( | |
) | ||
|
||
var errUnrecognizedEncoding = fmt.Errorf("unrecognized encoding") | ||
var errUnrecognizedKeyData = fmt.Errorf("unrecognized key_data") | ||
|
||
// kafkaTracesProducer uses sarama to produce trace messages to Kafka. | ||
type kafkaTracesProducer struct { | ||
|
@@ -178,8 +179,11 @@ func newMetricsExporter(config Config, set exporter.CreateSettings, marshalers m | |
|
||
// newTracesExporter creates Kafka exporter. | ||
func newTracesExporter(config Config, set exporter.CreateSettings, marshalers map[string]TracesMarshaler) (*kafkaTracesProducer, error) { | ||
marshaler := marshalers[config.Encoding] | ||
marshaler := marshalers[KeyOfTracerMarshallerBy(config.Encoding, config.KeyData)] | ||
if marshaler == nil { | ||
if config.KeyData != "none" && config.KeyData != "traceID" { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ideally this should be done within the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i wanted to make least changes needed, and mechanism of validation was done in that place so i kept it here. |
||
return nil, errUnrecognizedKeyData | ||
} | ||
return nil, errUnrecognizedEncoding | ||
} | ||
producer, err := newSaramaProducer(config) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,6 +8,9 @@ import ( | |
"go.opentelemetry.io/collector/pdata/plog" | ||
"go.opentelemetry.io/collector/pdata/pmetric" | ||
"go.opentelemetry.io/collector/pdata/ptrace" | ||
|
||
"github.com/open-telemetry/opentelemetry-collector-contrib/internal/coreinternal/traceutil" | ||
"github.com/open-telemetry/opentelemetry-collector-contrib/pkg/batchpersignal" | ||
) | ||
|
||
type pdataLogsMarshaler struct { | ||
|
@@ -90,9 +93,49 @@ func (p pdataTracesMarshaler) Encoding() string { | |
return p.encoding | ||
} | ||
|
||
func (p pdataTracesMarshaler) KeyData() string { | ||
return "none" | ||
} | ||
|
||
type pdataTracesMarshalerByTraceId pdataTracesMarshaler | ||
|
||
func (p pdataTracesMarshalerByTraceId) Marshal(td ptrace.Traces, topic string) ([]*sarama.ProducerMessage, error) { | ||
var messages []*sarama.ProducerMessage | ||
|
||
for _, tracesById := range batchpersignal.SplitTraces(td) { | ||
bts, err := p.marshaler.MarshalTraces(tracesById) | ||
if err != nil { | ||
return nil, err | ||
} | ||
var traceID = tracesById.ResourceSpans().At(0).ScopeSpans().At(0).Spans().At(0).TraceID() | ||
key := traceutil.TraceIDToHexOrEmptyString(traceID) | ||
messages = append(messages, &sarama.ProducerMessage{ | ||
Topic: topic, | ||
Value: sarama.ByteEncoder(bts), | ||
Key: sarama.ByteEncoder(key), | ||
}) | ||
} | ||
return messages, nil | ||
} | ||
|
||
func (p pdataTracesMarshalerByTraceId) Encoding() string { | ||
return p.encoding | ||
} | ||
|
||
func (p pdataTracesMarshalerByTraceId) KeyData() string { | ||
return "traceID" | ||
} | ||
|
||
func newPdataTracesMarshaler(marshaler ptrace.Marshaler, encoding string) TracesMarshaler { | ||
return pdataTracesMarshaler{ | ||
marshaler: marshaler, | ||
encoding: encoding, | ||
} | ||
} | ||
|
||
func newPdataTracesMarshalerByTraceId(marshaler ptrace.Marshaler, encoding string) TracesMarshaler { | ||
return pdataTracesMarshalerByTraceId{ | ||
marshaler: marshaler, | ||
encoding: encoding, | ||
} | ||
Comment on lines
+100
to
+140
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please move this new code contributions to a new file to make it easier to maintain. |
||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would say that
""
is also the same asnone
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please also add this to the
Validation
method as well.