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

Add test to verify that ConsumeMetrics and ConsumeTraces clear response members #1975

Merged
merged 2 commits into from
Jul 28, 2024

Conversation

ddelnano
Copy link
Member

@ddelnano ddelnano commented Jul 26, 2024

Summary: Add test to verify that ConsumeMetrics and ConsumeTraces clear response members

This adds test coverage for the bug fix in #1910. This is a follow up to the conversation here

Relevant Issues: N/A

Type of change: /kind bug

Test Plan: Verified that unit test fails if #1910 is reverted

$ git show HEAD
commit 4ab4a9ce58bd394e4cd8287b44767e870fedd127 (HEAD -> ddelnano/add-tests-for-otel-sink-bug, ddelnano/ddelnano/add-tests-for-otel-sink-bug)
Author: Dom Del Nano <[email protected]>
Date:   Fri Jul 26 12:17:00 2024 +0000

    Revert "Clear trace response instead of metric response in `OTelExportSinkNode::ConsumeSpans` (#1910)"

    This reverts commit 970a54a7b9deb9469c406d73069c1e310a53ea96.

$ bazel test src/carnot/exec:otel_export_sink_node_test --test_output=all

[ ... ]
[ RUN      ] OTelExportSinkNodeTest.consume_spans_clears_span_responses
src/carnot/exec/otel_export_sink_node_test.cc:1748: Failure
Value of: response->partial_success().rejected_spans() == 0
  Actual: false
Expected: true

@ddelnano ddelnano requested a review from a team as a code owner July 26, 2024 12:23
Signed-off-by: Dom Del Nano <[email protected]>
@ddelnano ddelnano force-pushed the ddelnano/add-tests-for-otel-sink-bug branch from 48b8d92 to d1af5ea Compare July 26, 2024 12:29
@ddelnano ddelnano merged commit b01f8ae into pixie-io:main Jul 28, 2024
29 checks passed
@ddelnano ddelnano deleted the ddelnano/add-tests-for-otel-sink-bug branch July 28, 2024 21:34
ddelnano added a commit to ddelnano/pixie that referenced this pull request Sep 23, 2024
…se members (pixie-io#1975)

Summary: Add test to verify that ConsumeMetrics and ConsumeTraces clear
response members

This adds test coverage for the bug fix in pixie-io#1910. This is a follow up to
the conversation
[here](pixie-io#1910 (comment))

Relevant Issues: N/A

Type of change: /kind bug

Test Plan: Verified that unit test fails if pixie-io#1910 is reverted
```
$ git show HEAD
commit 4ab4a9c (HEAD -> ddelnano/add-tests-for-otel-sink-bug, ddelnano/ddelnano/add-tests-for-otel-sink-bug)
Author: Dom Del Nano <[email protected]>
Date:   Fri Jul 26 12:17:00 2024 +0000

    Revert "Clear trace response instead of metric response in `OTelExportSinkNode::ConsumeSpans` (pixie-io#1910)"

    This reverts commit 970a54a.

$ bazel test src/carnot/exec:otel_export_sink_node_test --test_output=all

[ ... ]
[ RUN      ] OTelExportSinkNodeTest.consume_spans_clears_span_responses
src/carnot/exec/otel_export_sink_node_test.cc:1748: Failure
Value of: response->partial_success().rejected_spans() == 0
  Actual: false
Expected: true

```

---------

Signed-off-by: Dom Del Nano <[email protected]>
GitOrigin-RevId: b01f8ae
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.

2 participants