Skip to content

Allow multiple calls to SetStatisticsHandler / SetLogHandler / SetErrorHandler#2155

Open
Guillaume Delahaye (g7ed6e) wants to merge 4 commits intoconfluentinc:masterfrom
g7ed6e:feature/multiple-calls-to-set-xxx-handler
Open

Allow multiple calls to SetStatisticsHandler / SetLogHandler / SetErrorHandler#2155
Guillaume Delahaye (g7ed6e) wants to merge 4 commits intoconfluentinc:masterfrom
g7ed6e:feature/multiple-calls-to-set-xxx-handler

Conversation

@g7ed6e
Copy link
Copy Markdown

Resolves #2154

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

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

Comment thread src/Confluent.Kafka/ConsumerBuilder.cs Outdated
Comment thread .gitignore
Comment thread src/Confluent.Kafka/ConsumerBuilder.cs Outdated
@g7ed6e Guillaume Delahaye (g7ed6e) force-pushed the feature/multiple-calls-to-set-xxx-handler branch 2 times, most recently 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
Copy Markdown

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
Copy Markdown
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
Copy Markdown

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
Copy Markdown
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.

@g7ed6e Guillaume Delahaye (g7ed6e) force-pushed the feature/multiple-calls-to-set-xxx-handler branch from 6e1e71e to e5552df Compare July 31, 2024 09:25
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