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.

Sign up for GitHub

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

Jump to bottom

Add receiver/consumer contract test helper #7516

Merged
merged 6 commits into from
Jun 1, 2023
Merged

Add receiver/consumer contract test helper #7516

merged 6 commits into from
Jun 1, 2023

Conversation

tigrannajaryan
Copy link
Member

  • Added a CheckConsumeContract() helper func that can be used to test the contract that receivers are expected to maintain for Consume() calls.
  • Added an example usage of CheckConsumeContract() helper.

Resolves #7478

@tigrannajaryan tigrannajaryan requested review from a team, bogdandrutu and dmitryax April 11, 2023 21:12
@tigrannajaryan
Copy link
Member Author

Let me know if a changelog entry is needed for this.

@codecov
Copy link

codecov bot commented Apr 11, 2023 •
edited
Loading

Codecov Report

Patch coverage: 73.98% and project coverage change: -0.29 ⚠️

Comparison is base (87dd85a) 91.02% compared to head (7eb8b8b) 90.74%.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #7516      +/-   ##
==========================================
- Coverage   91.02%   90.74%   -0.29%     
==========================================
  Files         295      296       +1     
  Lines       14535    14781     +246     
==========================================
+ Hits        13231    13413     +182     
- Misses       1042     1092      +50     
- Partials      262      276      +14
Impacted Files Coverage Δ
receiver/receivertest/contract_checker.go 73.98% <73.98%> (ø)

... and 2 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@tigrannajaryan
Copy link
Member Author

@open-telemetry/collector-approvers please review.

@tigrannajaryan tigrannajaryan mentioned this pull request Apr 25, 2023
Merged
@tigrannajaryan
Copy link
Member Author

@open-telemetry/collector-approvers please review.

1 similar comment
@tigrannajaryan
Copy link
Member Author

@open-telemetry/collector-approvers please review.

djaglowski
djaglowski reviewed May 9, 2023
View reviewed changes
Copy link
Member

@djaglowski djaglowski left a comment

Choose a reason for hiding this comment

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

This generally looks good to me. In addition to adding support for metrics and filling in test coverage somewhat, do you think it would make sense that this PR should include a real usage of this functionality, perhaps in testing the otlp receiver?

receiver/receivertest/contract_checker.go Outdated Show resolved Hide resolved
@tigrannajaryan
Copy link
Member Author

This generally looks good to me. In addition to adding support for metrics and filling in test coverage somewhat, do you think it would make sense that this PR should include a real usage of this functionality, perhaps in testing the otlp receiver?

I was trying to keep the PR small. Definitely planning to add an otlp receiver test in a future PR, but can add to this one if that's what we want to see.

@djaglowski
Copy link
Member

It's fine with me to limit this to only the receivertest package.

@djaglowski djaglowski added the Skip Changelog PRs that do not require a CHANGELOG.md entry label May 9, 2023
@tigrannajaryan
Copy link
Member Author

It's fine with me to limit this to only the receivertest package.

There is obviously the risk that the receivertest may need modifications to work for otlp receiver. I tried to foresee how it should work but no guarantees.

@github-actions
Copy link
Contributor

This PR was marked stale due to lack of activity. It will be closed in 14 days.

@github-actions github-actions bot added the Stale label May 24, 2023
@dmitryax dmitryax removed the Stale label May 24, 2023
dmitryax
dmitryax reviewed May 26, 2023
View reviewed changes
receiver/receivertest/contract_checker.go Outdated Show resolved Hide resolved
receiver/receivertest/contract_checker.go Outdated Show resolved Hide resolved
receiver/receivertest/contract_checker.go Outdated Show resolved Hide resolved
receiver/receivertest/contract_checker_test.go Outdated Show resolved Hide resolved
receiver/receivertest/contract_checker.go Outdated Show resolved Hide resolved
@tigrannajaryan
Add receiver/consumer contract test helper
a9a8ba4 
- Added a CheckConsumeContract() helper func that can be used to test the contract that
  receivers are expected to maintain for Consume() calls.
- Added an example usage of CheckConsumeContract() helper.

Resolves open-telemetry#7478
@tigrannajaryan
Address PR comments
f4ab02a
@dmitryax dmitryax mentioned this pull request May 26, 2023
Merged
dmitryax
dmitryax reviewed May 26, 2023
View reviewed changes
receiver/receivertest/contract_checker.go Outdated Show resolved Hide resolved
receiver/receivertest/contract_checker.go Outdated Show resolved Hide resolved
receiver/receivertest/contract_checker.go Show resolved Hide resolved
receiver/receivertest/contract_checker.go Outdated Show resolved Hide resolved
dmitryax
dmitryax reviewed Jun 1, 2023
View reviewed changes
receiver/receivertest/contract_checker.go Outdated Show resolved Hide resolved
receiver/receivertest/contract_checker.go Outdated Show resolved Hide resolved
dmitryax
dmitryax reviewed Jun 1, 2023
View reviewed changes
receiver/receivertest/contract_checker.go Outdated Show resolved Hide resolved
receiver/receivertest/contract_checker.go Outdated Show resolved Hide resolved
dmitryax
dmitryax approved these changes Jun 1, 2023
View reviewed changes
Copy link
Member

@dmitryax dmitryax left a comment

Choose a reason for hiding this comment

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

Thank you!

@dmitryax dmitryax merged commit deffd48 into open-telemetry:main Jun 1, 2023
@github-actions github-actions bot added this to the next release milestone Jun 1, 2023
@tigrannajaryan tigrannajaryan deleted the feature/tigran/rcverrtest branch June 2, 2023 01:29
@omrozowicz-splunk omrozowicz-splunk mentioned this pull request Aug 1, 2023
Merged
dmitryax added a commit that referenced this pull request Oct 24, 2023
@omrozowicz-splunk @dmitryax
[chore] add test helper to verify exporter behavior on errors (#8143)
e97ceca 
**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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@djaglowski djaglowski djaglowski left review comments

@dmitryax dmitryax dmitryax approved these changes

@bogdandrutu bogdandrutu Awaiting requested review from bogdandrutu

Assignees
No one assigned
Labels
Skip Changelog PRs that do not require a CHANGELOG.md entry
Projects
None yet
Milestone
v0.79.0
Development

Successfully merging this pull request may close these issues.

Add test helper to verify receiver behavior on errors
3 participants
@tigrannajaryan @djaglowski @dmitryax