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/sqlquery] add basic functionality #10867

Merged
merged 7 commits into from
Jun 28, 2022
Merged

[receiver/sqlquery] add basic functionality #10867

merged 7 commits into from
Jun 28, 2022

Conversation

pmcollins
Copy link
Member

@pmcollins pmcollins commented Jun 9, 2022

This is the second PR for the new sqlquery receiver, containing the receiver's functionality, but without enabling the component.

First PR is here: #9407

@pmcollins pmcollins marked this pull request as ready for review June 9, 2022 22:01
@pmcollins pmcollins requested a review from a team June 9, 2022 22:01
@pmcollins pmcollins requested a review from dmitryax as a code owner June 9, 2022 22:01
receiver/sqlqueryreceiver/config_test.go Outdated Show resolved Hide resolved
receiver/sqlqueryreceiver/integration_test.go Outdated Show resolved Hide resolved
receiver/sqlqueryreceiver/integration_test.go Outdated Show resolved Hide resolved
receiver/sqlqueryreceiver/metrics.go Outdated Show resolved Hide resolved
receiver/sqlqueryreceiver/db_client.go Outdated Show resolved Hide resolved
receiver/sqlqueryreceiver/receiver_test.go Show resolved Hide resolved
Copy link
Member

@crobert-1 crobert-1 left a comment

Choose a reason for hiding this comment

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

Looks good, thanks for making all of the updates! (Just need to update dependencies to fix the failing check)

Copy link
Member

@crobert-1 crobert-1 left a comment

Choose a reason for hiding this comment

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

Thanks for making the extra updates!

@Konig-Corey
Copy link
Contributor

Resolves feature request: #10544

receiver/sqlqueryreceiver/README.md Outdated Show resolved Hide resolved
receiver/sqlqueryreceiver/README.md Outdated Show resolved Hide resolved
@Konig-Corey
Copy link
Contributor

@pmcollins Created a PR against your branch that includes requested changes from @dmitryax as well as discussed tags addition. https://github.com/pmcollins/opentelemetry-collector-contrib/pull/2805

@dmitryax
Copy link
Member

dmitryax commented Jun 28, 2022

@Konig-Corey I believe you can push commits right into pmcollins:sqlreceiver branch. This also needs to be rebased

@Konig-Corey
Copy link
Contributor

@Konig-Corey I believe you can push commits right into pmcollins:sqlreceiver branch. This also needs to be rebased

I had tried that originally and didn't let me. I doubt it would be set to public access so I just created a PR. If he approves it it should just update this PR since its on the source branch of this PR.

Am I missing something?

@dmitryax
Copy link
Member

I'm not sure, I worked for me every time I tried.

Use 'float' instead of 'double' in config.
Disallow explicit aggregation when data type is gauge.
@dmitryax dmitryax merged commit dcf15a4 into open-telemetry:main Jun 28, 2022
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.

6 participants