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

Allow multiple calls to SetStatisticsHandler / SetLogHandler / SetErrorHandler #2155

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

g7ed6e
Copy link

@g7ed6e g7ed6e commented Dec 12, 2023

Resolves #2154

@g7ed6e g7ed6e force-pushed the feature/multiple-calls-to-set-xxx-handler branch from cf970f9 to 254ccfc Compare December 12, 2023 06:44
@g7ed6e g7ed6e changed the title Feature/multiple calls to set xxx handler Allow multiple calls to SetStatisticsHandler / SetLogHandler / SetErrorHandler Dec 14, 2023
@g7ed6e
Copy link
Author

g7ed6e commented Dec 14, 2023

@forlack: This PR would be useful for dotnet/aspire#951 and open-telemetry/opentelemetry-dotnet-contrib#1493. Please see details in related issue #2154

src/Confluent.Kafka/ConsumerBuilder.cs Outdated Show resolved Hide resolved
.gitignore Show resolved Hide resolved
src/Confluent.Kafka/ConsumerBuilder.cs Outdated Show resolved Hide resolved
@g7ed6e g7ed6e force-pushed the feature/multiple-calls-to-set-xxx-handler branch from f0d501a to 2694696 Compare December 14, 2023 20:37
@g7ed6e g7ed6e force-pushed the feature/multiple-calls-to-set-xxx-handler branch from 2694696 to 00a8164 Compare December 16, 2023 10:56
LogToFile("end ConsumerBuilder_SetStatisticsHandler");
}

[SkipWhenCITheory("Requires to stop the broker in the while loop to simulate broker is down."), MemberData(nameof(KafkaParameters))]
Copy link

Choose a reason for hiding this comment

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

I'm not sure I understand. This requires someone to manually stop the broker?

Copy link
Author

Choose a reason for hiding this comment

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

Exactly. this is required during the unit test to get the error handler called.

var record = consumer.Consume(TimeSpan.FromMilliseconds(100));
if (record == null) { continue; }
if (record.IsPartitionEOF) { break; }
msgCnt += 1;
Copy link

Choose a reason for hiding this comment

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

Do you want to assert how many messages were counted?

Copy link
Author

Choose a reason for hiding this comment

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

No that's not required here. good catch, I dropped the useless msgCnt variables.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow multiple calls to SetStatisticsHandler / SetLogHandler / SetErrorHandler
2 participants