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

[receiver/oracledbreceiver] add implementation of oracledbreceiver #16044

Merged
merged 7 commits into from
Dec 8, 2022

Conversation

atoulme
Copy link
Contributor

@atoulme atoulme commented Nov 3, 2022

Description:
Initial implementation of the oracledbreceiver.

Link to tracking Issue:
#16043

Testing:
Unit testing. Also performed extensive manual testing.

Documentation:
README.md

@atoulme atoulme requested a review from a team November 3, 2022 01:02
@atoulme atoulme requested a review from dmitryax as a code owner November 3, 2022 01:02
@atoulme atoulme force-pushed the oracledbreceiver_impl branch 2 times, most recently from cb1f9bf to 5d32b60 Compare November 3, 2022 01:27
return nil
}

func (s *scraper) Scrape(ctx context.Context) (pmetric.Metrics, error) {
Copy link
Member

Choose a reason for hiding this comment

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

There's kind of a lot of orthogonal functionality going on in this scraper. What do you think about splitting it up, perhaps into multiple, relatively small scrapers that get registered at startup time based on metric settings? That way, we'd end up with a few small types/functions and we won't have to checking setting on every scrape.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think having a single scraper is probably preferrable to send all metrics in one scrape and use at most one DB connection. The code could be nicer to look at, and refactored into smaller functions and appended as an array of calls that are set at start time if the setting is enabled.

@github-actions
Copy link
Contributor

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

@runforesight
Copy link

runforesight bot commented Dec 4, 2022

Foresight Summary

    
Major Impacts

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

⭕  build-and-test-windows workflow has finished in 4 seconds (38 minutes 4 seconds less than main branch avg.) and finished at 8th Dec, 2022.


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

✅  tracegen workflow has finished in 1 minute 2 seconds (2 minutes 23 seconds less than main branch avg.) and finished at 8th Dec, 2022.


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

✅  check-links workflow has finished in 1 minute 17 seconds (1 minute 41 seconds less than main branch avg.) and finished at 8th Dec, 2022.


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

✅  changelog workflow has finished in 2 minutes 14 seconds (2 minutes 55 seconds less than main branch avg.) and finished at 8th Dec, 2022.


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

✅  build-and-test workflow has finished in 47 minutes 38 seconds (15 minutes 18 seconds less than main branch avg.) and finished at 8th Dec, 2022. There are 2 test failures.


Job Failed Steps Tests
correctness-metrics -     🔗  ✅ 2  ❌ 0  ⏭ 0    🔗 See Details
correctness-traces -     🔗  ✅ 17  ❌ 0  ⏭ 0    🔗 See Details
unittest-matrix (1.19, internal) -     🔗  ✅ 592  ❌ 0  ⏭ 0    🔗 See Details
unittest-matrix (1.18, internal) -     🔗  ✅ 592  ❌ 0  ⏭ 0    🔗 See Details
unittest-matrix (1.19, processor) -     🔗  ✅ 1465  ❌ 0  ⏭ 0    🔗 See Details
unittest-matrix (1.18, extension) -     🔗  ✅ 528  ❌ 0  ⏭ 0    🔗 See Details
unittest-matrix (1.19, extension) -     🔗  ✅ 528  ❌ 0  ⏭ 0    🔗 See Details
unittest-matrix (1.18, processor) -     🔗  ✅ 1465  ❌ 0  ⏭ 0    🔗 See Details
unittest-matrix (1.18, receiver-0) -     🔗  ✅ 2533  ❌ 0  ⏭ 0    🔗 See Details
unittest-matrix (1.19, receiver-0) -     🔗  ✅ 2533  ❌ 0  ⏭ 0    🔗 See Details
unittest-matrix (1.18, other) -     🔗  ✅ 4359  ❌ 0  ⏭ 0    🔗 See Details
unittest-matrix (1.18, receiver-1) -     🔗  ✅ 1851  ❌ 0  ⏭ 0    🔗 See Details
unittest-matrix (1.18, exporter) -     🔗  ✅ 2415  ❌ 2  ⏭ 0    🔗 See Details
unittest-matrix (1.19, receiver-1) -     🔗  ✅ 1851  ❌ 0  ⏭ 0    🔗 See Details
unittest-matrix (1.19, exporter) -     🔗  ✅ 2417  ❌ 0  ⏭ 0    🔗 See Details
integration-tests -     🔗  ✅ 57  ❌ 0  ⏭ 0    🔗 See Details
unittest-matrix (1.19, other) -     🔗  ✅ 4359  ❌ 0  ⏭ 0    🔗 See Details
setup-environment -     🔗  N/A See Details
checks -     🔗  N/A See Details
build-examples -     🔗  N/A See Details
check-collector-module-version -     🔗  N/A See Details
check-codeowners -     🔗  N/A See Details
lint-matrix (receiver-0) -     🔗  N/A See Details
lint-matrix (receiver-1) -     🔗  N/A See Details
lint-matrix (processor) -     🔗  N/A See Details
lint-matrix (exporter) -     🔗  N/A See Details
lint-matrix (extension) -     🔗  N/A See Details
lint-matrix (internal) -     🔗  N/A See Details
lint-matrix (other) -     🔗  N/A See Details
unittest (1.19) -     🔗  N/A See Details
unittest (1.18) -     🔗  N/A See Details
lint -     🔗  N/A See Details
cross-compile (darwin, amd64) -     🔗  N/A See Details
cross-compile (darwin, arm64) -     🔗  N/A See Details
cross-compile (linux, 386) -     🔗  N/A See Details
cross-compile (linux, amd64) -     🔗  N/A See Details
cross-compile (linux, arm) -     🔗  N/A See Details
cross-compile (linux, arm64) -     🔗  N/A See Details
cross-compile (linux, ppc64le) -     🔗  N/A See Details
cross-compile (windows, 386) -     🔗  N/A See Details
cross-compile (windows, amd64) -     🔗  N/A See Details
build-package (rpm) -     🔗  N/A See Details
windows-msi -     🔗  N/A See Details
build-package (deb) -     🔗  N/A See Details
publish-check -     🔗  N/A See Details
publish-stable -     🔗  N/A See Details
publish-dev -     🔗  N/A See Details

✅  prometheus-compliance-tests workflow has finished in 10 minutes 54 seconds (⚠️ 1 minute 43 seconds more than main branch avg.) and finished at 8th Dec, 2022.


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

✅  load-tests workflow has finished in 16 minutes 28 seconds and finished at 8th Dec, 2022.


Job Failed Steps Tests
loadtest (TestTraceAttributesProcessor) -     🔗  ✅ 3  ❌ 0  ⏭ 0    🔗 See Details
loadtest (TestIdleMode) -     🔗  ✅ 1  ❌ 0  ⏭ 0    🔗 See Details
loadtest (TestMetric10kDPS|TestMetricsFromFile) -     🔗  ✅ 6  ❌ 0  ⏭ 0    🔗 See Details
loadtest (TestTraceNoBackend10kSPS|TestTrace1kSPSWithAttrs) -     🔗  ✅ 8  ❌ 0  ⏭ 0    🔗 See Details
loadtest (TestMetricResourceProcessor|TestTrace10kSPS) -     🔗  ✅ 12  ❌ 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.

@github-actions github-actions bot removed the Stale label Dec 5, 2022
receiver/oracledbreceiver/scraper.go Outdated Show resolved Hide resolved
receiver/oracledbreceiver/factory.go Outdated Show resolved Hide resolved
receiver/oracledbreceiver/scraper.go Outdated Show resolved Hide resolved
@atoulme atoulme force-pushed the oracledbreceiver_impl branch 2 times, most recently from ce05fde to 6b75049 Compare December 7, 2022 07:14
@atoulme atoulme force-pushed the oracledbreceiver_impl branch from 6b75049 to e1d0b31 Compare December 7, 2022 07:23
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.

Just couple of nits

receiver/oracledbreceiver/scraper.go Outdated Show resolved Hide resolved
receiver/oracledbreceiver/testdata/config.yaml Outdated Show resolved Hide resolved
@atoulme atoulme force-pushed the oracledbreceiver_impl branch from 0362947 to 26ec7ff Compare December 8, 2022 05:19
@dmitryax dmitryax merged commit 0f2b033 into open-telemetry:main Dec 8, 2022
@atoulme atoulme deleted the oracledbreceiver_impl branch December 8, 2022 07:40
@plantfansam plantfansam mentioned this pull request Jul 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants