Address comments from PR 3625 #3633
Conversation
kkondaka
requested review from
chenqi0805,
engechas,
graytaylor0,
dinujoh,
asifsmohammed,
dlvenable and
oeyh
as code owners
| import static org.hamcrest.Matchers.notNullValue; | ||
| import static org.mockito.Mockito.mock; | ||
| import static org.mockito.Mockito.when; | ||
|
|
||
| @ExtendWith(MockitoExtension.class) | ||
| public class KafkaBufferOTelIT { | ||
| private static final String TEST_REQUEST_MULTIPLE_METRICS_FILE = "test-request-multiple-metrics.json"; | ||
| private static final String TEST_REQUEST_MULTIPLE_TRACES_FILE = "test-request-multiple-traces.json"; |
There was a problem hiding this comment.
These tests are mixing two very distinct components - OTel binary processing and the Kafka buffer. I tend to think that this test should take arbitrary binary data.
For testing the Kafka buffer with OTel, I think we could add a variation on the existing end-to-end tests which runs the same tests, but using the Kafka buffer instead of the bounding_buffer buffer. This would be fairly reasonable because we could use the existing tests. We just swap out the buffer for the different tests. Those tests already run Docker images as well.
There was a problem hiding this comment.
Modified to use arbitrary data instead of data from the file.
For the end-to-end tests, I will do a different PR
Signed-off-by: Krishna Kondaka <[email protected]>
Signed-off-by: Krishna Kondaka <[email protected]>
kkondaka
force-pushed
the
pr-3625-comments
branch
from
371d5c7 to
8782155
Compare
Signed-off-by: Krishna Kondaka <[email protected]>
|
@asifsmohammed rebased and fixed the build failure issue. |
Description
Address comments made in PR 3625
Issues Resolved
Resolves #[Issue number to be closed when this PR is merged]
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.