messaging.kafka.bootstrap.servers added for KafkaProducer#17065
messaging.kafka.bootstrap.servers added for KafkaProducer#17065onurkybsi wants to merge 6 commits intoopen-telemetry:mainfrom
Conversation
onurkybsi
force-pushed
the
kafka-bootstrap-servers
branch
from
6484a3b to
d125e22
Compare
| if (request.getBootstrapServers() != null) { | ||
| attributes.put(MESSAGING_KAFKA_BOOTSTRAP_SERVERS, request.getBootstrapServers()); |
There was a problem hiding this comment.
attributes.put ignores null values so null check isn't needed
our convention is that we don't emit non-spec attributes in default configuration. For example see https://github.com/open-telemetry/opentelemetry-java-instrumentation/blob/main/instrumentation/kafka/kafka-clients/kafka-clients-common-0.11/library/src/main/java/io/opentelemetry/instrumentation/kafkaclients/common/v0_11/internal/KafkaConsumerExperimentalAttributesExtractor.java
|
|
||
| /** Sets Kafka consumer configurations. */ | ||
| @CanIgnoreReturnValue | ||
| public KafkaTelemetryBuilder setProducerConfigs(Map<String, Object> producerConfigs) { |
There was a problem hiding this comment.
this is not ideal, currently the same KafkaTelemetry instance could be used to add telemetry to different producers but with this it is not the case any more. Did you consider alternatives like using reflection to read the producerConfig field in KafkaProducer?
onurkybsi
force-pushed
the
kafka-bootstrap-servers
branch
from
c13074b to
f8727e0
Compare
# Conflicts: # instrumentation/kafka/kafka-clients/kafka-clients-2.6/library/src/main/java/io/opentelemetry/instrumentation/kafkaclients/v2_6/KafkaTelemetryBuilder.java # Conflicts: # instrumentation/spring/spring-kafka-2.7/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/spring/kafka/v2_7/SpringKafkaTest.java
onurkybsi
force-pushed
the
kafka-bootstrap-servers
branch
from
f8727e0 to
6514ffb
Compare
| val -> val.isInstanceOf(String.class)), | ||
| equalTo(MESSAGING_KAFKA_MESSAGE_OFFSET, 0), | ||
| equalTo(MESSAGING_KAFKA_MESSAGE_KEY, "10"))); | ||
| if (isExperimental) { |
There was a problem hiding this comment.
you could instead use
satisfies(
MESSAGING_KAFKA_BOOTSTRAP_SERVERS,
stringAssert -> {
if (isExperimental) {
stringAssert.matches("^localhost:\\d+(,localhost:\\d+)*$");
}
}));
then you wouldn't need the assertions list
2 participants
messaging.kafka.bootstrap.serversattribute added for instrumentedKafkaProducer, see #10647.