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/saphanareceiver] New SAP HANA metrics receiver #9234

Merged
merged 62 commits into from
May 2, 2022

Conversation

dehaansa
Copy link
Contributor

Description:
Adds a new SAP HANA metrics receiver.

Link to tracking Issue: #8827

Testing: Local testing & unit testing. Integration tests were attempted, but could not get the sap hana test docker image running within testcontainers.

@dehaansa dehaansa requested a review from a team April 12, 2022 17:24
@dehaansa dehaansa requested a review from dmitryax as a code owner April 28, 2022 15:14
rowFields := make([]interface{}, 0)

// Build a list of addresses that rows.Scan will load column data into
for range query.orderedResourceLabels {
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps instead of iterating here, as well orderedMetricLabels and orderedStats below, rowFields could be created with single make? (since we know the sizes of each)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

rowFields needs to be a set of pointers to objects, so it needs more than just a singular make in my understanding, but I was able to update it to be a little bit cleaner.

@pmm-sumo
Copy link
Contributor

Some nits, it looks good overall. I will make another run tomorrow

Copy link
Contributor

@pmm-sumo pmm-sumo left a comment

Choose a reason for hiding this comment

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

This is a really large PR and I'm sure it required a fair amount of work. Thank you for the contribution! I think it looks good overall. Another pair of eyes would be helpful before merging this

CHANGELOG.md Outdated Show resolved Hide resolved
@djaglowski djaglowski merged commit e7b5ac7 into open-telemetry:main May 2, 2022
@dehaansa dehaansa deleted the sap-hana branch May 2, 2022 16:01
djaglowski added a commit to djaglowski/opentelemetry-collector-contrib that referenced this pull request May 10, 2022
…try#9234)

* initial commit

* initial generated metric

* First sap hana metric

* Fully functioning first metric

* Set up structures for monitoring queries for sap hana

* Clean up sap hana collection code a bit

* More touchups to hana

* initial tests & enhancements to support unit tests

* Small fixes, finish metadata

* Add rest of metrics to metadata

* Enhance null support, finish adding queries

* Enhance testing & update queries

* tidy up the gomod

* Support disabling queries

* Update queries slightly

* Ensure variable coming from go sql is not ratio

* Update saphana documentation

* Alphabetize sap hana queries in LPU documentation

* Minor change to sap hana documentation

* Update go mod

* Update query based on feedback

* Fix documentation

* Address linter issues

* Add changelog entry

* Appease linter

* Add component to components list for saphana

* Update receiver/saphanareceiver/Makefile

Co-authored-by: Daniel Jaglowski <jaglows3@gmail.com>

* Update receiver/saphanareceiver/testdata/mocked_queries/mostly_disabled_results.json

Co-authored-by: Daniel Jaglowski <jaglows3@gmail.com>

* Update receiver/saphanareceiver/testdata/mocked_queries/all_query_results.json

Co-authored-by: Daniel Jaglowski <jaglows3@gmail.com>

* Address some PR feedback

* Ensure all errors end up in ScrapeErrors

* Add codeowner

* Update dependencies in gomod

* Add resource attributes

* Address linter

* Update gomod

* Add db.system identifier

* Tiny change to error message to trigger ci

* Add replace to saphana go mod

* Update reference in saphana gomod

* Fix unnecessary update to sapmexporter gomod

* Address PR feedback

* Reduce repetition in client

* Simplify assignment in client

* Update gomod

* Fix another gomod entry

* Fix changelog location after merge

Co-authored-by: Daniel Jaglowski <jaglows3@gmail.com>
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.

3 participants