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 helper to verify exporter behavior on errors #7479

Closed
tigrannajaryan opened this issue Apr 3, 2023 · 5 comments
Closed

Add test helper to verify exporter behavior on errors #7479

tigrannajaryan opened this issue Apr 3, 2023 · 5 comments

Comments

@tigrannajaryan
Copy link
Member

The consumers (and all exporters are consumers) are required to follow the Error Handling contract which says:

// If the error is non-Permanent then the nextConsumer.Consume*() call should be retried
// with the same data.

and

// If the error is Permanent, then the nextConsumer.Consume*() call should not be
// retried with the same data.

We need a test helper that makes testing exporters for this part of the contract easy.

Helper Inputs

The helper should accept:

  • An exporter factory.
  • A config for the exporter. We may want to make it easy to allocate a listening port for mock receivers and pass it to the exporter config as the endpoint to send to.
  • A mock receiver. E.g. for otlp exporter testing we need otlp receiver that behaves correctly according to OTLP protocol spec and responds as instructed by the helper.

Events to Test for

It is important that all 3 event types happen during the test at least once:

  • ConsumeLogs/Traces/Metrics call succeeds at the first try.
  • ConsumeLogs/Traces/Metrics call fails once with non-permanent error, then succeeds on the next try.
  • ConsumeLogs/Traces/Metrics call fails more than once with non-permanent error, then eventually succeeds.
  • ConsumeLogs/Traces/Metrics call fails once with permanent error.

The test helper should provide a data generator that can create data to be given to the exporter.

Test Scenarios

The helper should be possible to configure for testing for permanent errors scenario and must verify:

  • No data is lost if mock receiver always succeeds.
  • No data is duplicated (in the presence or absence of mock receiver failures).
  • Data is not retried by the exporter when the mock receiver indicates a permanent error to the exporter.

The helper should be possible to configure for testing for non-permanent errors scenario and must verify:

  • No data is lost.
  • No data is duplicated.
  • Generated and delivered data content match exactly (but requests may be re-ordered).

Other Requirements

  • The test helper should generate concurrent calls to the exporter's ConsumeLogs/Traces/Metrics.
  • The mock receiver should be able to accept and respond to concurrent requests from the exporter.
  • The helper should support all 3 signal types: logs, traces, metrics.
@omrozowicz-splunk
Copy link
Contributor

Hey! I spent some time figuring out this task (mostly in comparision with the receiver test helper) and have a few questions:

  1. In the receiver tests the structure is generator -> receiver -> mockConsumer, here is it only exporter -> receiver?
  2. I see mock receiver already implemented here, would it make sense to reuse it? I specifically mean to use setExportResponse function to set the error.
  3. Do I understand it right that mockReceiver should implement similar functions to mockConsumer like acceptedAndDropped etc. Also, why does mockReceiver is really needed? Can't we use mockConsumer as well?
  4. Can I reuse generator (from receiver test helper) to generate the data for exporter to take?

CC: @VihasMakwana

@tigrannajaryan
Copy link
Member Author

  1. In the receiver tests the structure is generator -> receiver -> mockConsumer, here is it only exporter -> receiver?

Yes. I think the test helper itself can generate the necessary data and give to to exporter.

I see mock receiver already implemented here, would it make sense to reuse it? I specifically mean to use setExportResponse function to set the error.

Yes, it would be useful to reuse it. You may need to add ability to return specific gRPC errors so that you can trigger the permanent and non-permanent error scenarios.

Do I understand it right that mockReceiver should implement similar functions to mockConsumer like acceptedAndDropped etc. Also, why does mockReceiver is really needed? Can't we use mockConsumer as well?

No. mockReceiver should be an implementation of the actual network protocol that the exporter-under-test uses. In case of otlpexpoter testing mockReceiver should OTLP/gRPC protocol receiving (or OTLP/HTTP for testing of otlphttpexporter).

This is completely different from mockConsumer, which merely implements the internal Consumer interface.

Can I reuse generator (from receiver test helper) to generate the data for exporter to take?

I don't think it is necessary. The exporter test helper can generate the data itself. The generator does not need to be specific to the exporter.

@omrozowicz-splunk
Copy link
Contributor

Hey @tigrannajaryan, I opened a Draft. So far only logs are implemented, but if you confirm it is good direction I'll add traces and metrics and open a proper PR.

The behaviour of the exporter looks correct, this is the example of logs from a test suite I ran:

=== RUN   TestConsumeContract/always_succeed
Preparing log number:  0
Successfully sent log number: 0
Preparing log number:  1
Successfully sent log number: 1
Preparing log number:  2
Successfully sent log number: 2
Preparing log number:  3
Successfully sent log number: 3
Preparing log number:  4
Successfully sent log number: 4
Preparing log number:  5
Successfully sent log number: 5
Preparing log number:  6
Successfully sent log number: 6
Preparing log number:  7
Successfully sent log number: 7
Preparing log number:  8
Successfully sent log number: 8
Preparing log number:  9
Successfully sent log number: 9
Number of export tries: 10
Total items received successfully: 10
Number of errors: 0
    --- PASS: TestConsumeContract/always_succeed (0.01s)
=== RUN   TestConsumeContract/random_non_permanent_error
Preparing log number:  0
Successfully sent log number: 0
Preparing log number:  1
non-permament error happened
Retrying log number:  1
Successfully sent log number: 1
Preparing log number:  2
Successfully sent log number: 2
Preparing log number:  3
non-permament error happened
Retrying log number:  3
non-permament error happened
Retrying log number:  3
Successfully sent log number: 3
Preparing log number:  4
Successfully sent log number: 4
Preparing log number:  5
Successfully sent log number: 5
Preparing log number:  6
non-permament error happened
Retrying log number:  6
Successfully sent log number: 6
Preparing log number:  7
Successfully sent log number: 7
Preparing log number:  8
non-permament error happened
Retrying log number:  8
Successfully sent log number: 8
Preparing log number:  9
non-permament error happened
Retrying log number:  9
Successfully sent log number: 9
Number of export tries: 16
Total items received successfully: 10
Number of errors: 6
    --- PASS: TestConsumeContract/random_non_permanent_error (33.21s)
=== RUN   TestConsumeContract/random_permanent_error
Preparing log number:  0
permament error happened
Dropping log number:  0
Preparing log number:  1
Successfully sent log number: 1
Preparing log number:  2
Successfully sent log number: 2
Preparing log number:  3
permament error happened
Dropping log number:  3
Preparing log number:  4
permament error happened
Dropping log number:  4
Preparing log number:  5
Successfully sent log number: 5
Preparing log number:  6
Successfully sent log number: 6
Preparing log number:  7
permament error happened
Dropping log number:  7
Preparing log number:  8
Successfully sent log number: 8
Preparing log number:  9
Successfully sent log number: 9
Number of export tries: 10
Total items received successfully: 6
Number of errors: 4
    --- PASS: TestConsumeContract/random_permanent_error (0.01s)
=== RUN   TestConsumeContract/random_error
Preparing log number:  0
non-permament error happened
Retrying log number:  0
non-permament error happened
Retrying log number:  0
Preparing log number:  1
non-permament error happened
Retrying log number:  1
Preparing log number:  2
Successfully sent log number: 2
Preparing log number:  3
Successfully sent log number: 3
Preparing log number:  4
non-permament error happened
Retrying log number:  4
non-permament error happened
Retrying log number:  4
Preparing log number:  5
non-permament error happened
Retrying log number:  5
Preparing log number:  6
non-permament error happened
Retrying log number:  6
Successfully sent log number: 6
Preparing log number:  7
non-permament error happened
Retrying log number:  7
Preparing log number:  8
Successfully sent log number: 8
Preparing log number:  9
Successfully sent log number: 9
Number of export tries: 13
Total items received successfully: 5
Number of errors: 8
    --- PASS: TestConsumeContract/random_error (15.33s)
PASS

@tigrannajaryan
Copy link
Member Author

Hey @tigrannajaryan, I opened a Draft. So far only logs are implemented, but if you confirm it is good direction I'll add traces and metrics and open a proper PR.

@omrozowicz-splunk Thanks for working on this. I probably won't be able to review your PR since I will out next week but Collector maintainers/approvers should be able to provide feedback.

dmitryax added a commit that referenced this issue Oct 24, 2023
**Description:** Adding a test helper to test exporter behaviour on
errors.

**Link to tracking Issue:**
#7479
Will also fix:
#7481

It is created based on the exporter receiver test:
#7516
The exporter targets `mockReceiver` as an endpoint.

The test cases covered here are:
1. ConsumeLogs/Traces/Metrics call succeeds at the first try
2. ConsumeLogs/Traces/Metrics call fails once with non-permanent error,
then succeeds on one of the next tries
3. ConsumeLogs/Traces/Metrics call fails once with permanent error and
the data is not retried
4. ConsumeLogs/Traces/Metrics call fails once with permanent error or
permanent error (then the test pass requirement is calculated based on
number of errors per kind/successfully delivered data/total number of
data to be delivered)

---------

Co-authored-by: Dmitry Anoshin <anoshindx@gmail.com>
@dmitryax
Copy link
Member

Resolved by #8143

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

No branches or pull requests

3 participants