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

[connector/count] Add outline and documentation #18037

Merged
merged 7 commits into from
Jan 26, 2023

Conversation

djaglowski
Copy link
Member

First PR for #17912

@djaglowski djaglowski added the Skip Changelog PRs that do not require a CHANGELOG.md entry label Jan 25, 2023
@runforesight
Copy link

runforesight bot commented Jan 25, 2023

Foresight Summary

    
Major Impacts

build-and-test-windows duration(4 seconds) has decreased 40 minutes 37 seconds compared to main branch avg(40 minutes 41 seconds).
View More Details

⭕  changelog workflow has finished in 3 seconds (2 minutes 1 second less than main branch avg.) and finished at 26th Jan, 2023.


Job Failed Steps Tests
changelog -     🔗  N/A See Details

⭕  build-and-test-windows workflow has finished in 4 seconds (40 minutes 37 seconds less than main branch avg.) and finished at 26th Jan, 2023.


Job Failed Steps Tests
windows-unittest-matrix -     🔗  N/A See Details
windows-unittest -     🔗  N/A See Details

✅  telemetrygen workflow has finished in 47 seconds (1 minute 36 seconds less than main branch avg.) and finished at 26th Jan, 2023.


Job Failed Steps Tests
build-dev -     🔗  N/A See Details
publish-latest -     🔗  N/A See Details
publish-stable -     🔗  N/A See Details

✅  tracegen workflow has finished in 50 seconds (1 minute 34 seconds less than main branch avg.) and finished at 26th Jan, 2023.


Job Failed Steps Tests
build-dev -     🔗  N/A See Details
publish-latest -     🔗  N/A See Details
publish-stable -     🔗  N/A See Details

❌  check-links workflow has finished in 1 minute 22 seconds and finished at 26th Jan, 2023. 1 job failed.


Job Failed Steps Tests
changed files -     🔗  N/A See Details
check-links Run markdown-link-check     🔗  N/A See Details

 build-and-test workflow has finished in 10 minutes 58 seconds (41 minutes 4 seconds less than main branch avg.) and finished at 26th Jan, 2023.


Job Failed Steps Tests
unittest-matrix (1.19, internal) N/A  ✅ 561  ❌ 0  ⏭ 0    🔗 See Details
unittest-matrix (1.18, internal) N/A  ✅ 561  ❌ 0  ⏭ 0    🔗 See Details
unittest-matrix (1.19, processor) N/A  ✅ 1471  ❌ 0  ⏭ 0    🔗 See Details
correctness-metrics N/A  ✅ 2  ❌ 0  ⏭ 0    🔗 See Details
unittest-matrix (1.18, processor) N/A  ✅ 1471  ❌ 0  ⏭ 0    🔗 See Details
unittest-matrix (1.19, extension) N/A  ✅ 547  ❌ 0  ⏭ 0    🔗 See Details
unittest-matrix (1.18, extension) N/A  ✅ 547  ❌ 0  ⏭ 0    🔗 See Details
correctness-traces N/A  ✅ 17  ❌ 0  ⏭ 0    🔗 See Details
unittest-matrix (1.19, receiver-0) N/A  ✅ 2567  ❌ 0  ⏭ 0    🔗 See Details
unittest-matrix (1.18, receiver-0) N/A  ✅ 2567  ❌ 0  ⏭ 0    🔗 See Details
unittest-matrix (1.19, exporter) N/A  ✅ 2416  ❌ 0  ⏭ 0    🔗 See Details
unittest-matrix (1.18, exporter) N/A  ✅ 2416  ❌ 0  ⏭ 0    🔗 See Details
unittest-matrix (1.19, receiver-1) N/A  ✅ 1944  ❌ 0  ⏭ 0    🔗 See Details
unittest-matrix (1.18, other) N/A  ✅ 4661  ❌ 0  ⏭ 0    🔗 See Details
integration-tests N/A  ✅ 55  ❌ 0  ⏭ 0    🔗 See Details
unittest-matrix (1.18, receiver-1) N/A  ✅ 1944  ❌ 0  ⏭ 0    🔗 See Details
unittest-matrix (1.19, other) N/A  ✅ 4661  ❌ 0  ⏭ 0    🔗 See Details

✅  prometheus-compliance-tests workflow has finished in 3 minutes 40 seconds (3 minutes 49 seconds less than main branch avg.) and finished at 26th Jan, 2023.


Job Failed Steps Tests
prometheus-compliance-tests -     🔗  ✅ 21  ❌ 0  ⏭ 0    🔗 See Details

✅  load-tests workflow has finished in 7 minutes 55 seconds (6 minutes 8 seconds less than main branch avg.) and finished at 26th Jan, 2023.


Job Failed Steps Tests
loadtest (TestTraceAttributesProcessor) -     🔗  ✅ 3  ❌ 0  ⏭ 0    🔗 See Details
loadtest (TestIdleMode) -     🔗  ✅ 1  ❌ 0  ⏭ 0    🔗 See Details
loadtest (TestTraceNoBackend10kSPS|TestTrace1kSPSWithAttrs) -     🔗  ✅ 8  ❌ 0  ⏭ 0    🔗 See Details
loadtest (TestMetricResourceProcessor|TestTrace10kSPS) -     🔗  ✅ 12  ❌ 0  ⏭ 0    🔗 See Details
loadtest (TestMetric10kDPS|TestMetricsFromFile) -     🔗  ✅ 6  ❌ 0  ⏭ 0    🔗 See Details
loadtest (TestTraceBallast1kSPSWithAttrs|TestTraceBallast1kSPSAddAttrs) -     🔗  ✅ 10  ❌ 0  ⏭ 0    🔗 See Details
loadtest (TestBallastMemory|TestLog10kDPS) -     🔗  ✅ 19  ❌ 0  ⏭ 0    🔗 See Details
setup-environment -     🔗  N/A See Details

🔎 See details on Foresight

*You can configure Foresight comments in your organization settings page.

@djaglowski djaglowski marked this pull request as ready for review January 25, 2023 17:16
@djaglowski djaglowski requested review from a team and MovieStoreGuy January 25, 2023 17:16
@djaglowski djaglowski requested review from jpkrohling and atoulme and removed request for MovieStoreGuy January 25, 2023 17:16
@djaglowski
Copy link
Member Author

The readme is intended to introduce the component in a way that is almost a general introduction to connectors. In the not too distant future, I imagine pulling the very generalized parts into an upstream document and linking to that.

Copy link
Member

@jpkrohling jpkrohling left a comment

Choose a reason for hiding this comment

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

You could also include the following files in this PR:

  • .github/ISSUE_TEMPLATE/bug_report.yaml
  • .github/ISSUE_TEMPLATE/feature_request.yaml
  • .github/ISSUE_TEMPLATE/other.yaml
  • .github/dependabot.yml
  • versions.yaml
  • go.mod
  • .github/CODEOWNERS

connector/countconnector/README.md Show resolved Hide resolved
@@ -0,0 +1,213 @@
# Count Connector
Copy link
Member

Choose a reason for hiding this comment

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

I like how this readme is written, mostly demonstrating how connectors work. At the same time, I wonder if this wouldn't be confusing to users of this connector specifically.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks, I'm reworking this into a general connectors readme and a dedicated count connector readme.

connector/countconnector/factory.go Outdated Show resolved Hide resolved
@djaglowski
Copy link
Member Author

You could also include the following files in this PR:

  • .github/ISSUE_TEMPLATE/bug_report.yaml
  • .github/ISSUE_TEMPLATE/feature_request.yaml
  • .github/ISSUE_TEMPLATE/other.yaml
  • .github/dependabot.yml
  • versions.yaml
  • go.mod
  • .github/CODEOWNERS

Thanks, been a while since I added a new component. I'll include all of these.

@jpkrohling
Copy link
Member

Looks like we are missing some file, as the multimod-verify check failed:

Using versioning file /home/runner/work/opentelemetry-collector-contrib/opentelemetry-collector-contrib/versions.yaml
verifyAllModulesInSet failed: Module github.com/open-telemetry/opentelemetry-collector-contrib/connector/countconnector (defined in /home/runner/work/opentelemetry-collector-contrib/opentelemetry-collector-contrib/connector/countconnector/go.mod) is not listed in any module set.

Copy link
Contributor

@codeboten codeboten left a comment

Choose a reason for hiding this comment

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

Just one question otherwise it looks good.

connector/README.md Outdated Show resolved Hide resolved
Co-authored-by: Alex Boten <alex@boten.ca>
@djaglowski djaglowski merged commit 72d590e into open-telemetry:main Jan 26, 2023
@djaglowski djaglowski deleted the count-connector branch January 26, 2023 23:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Skip Changelog PRs that do not require a CHANGELOG.md entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants