-
Notifications
You must be signed in to change notification settings - Fork 239
Refactor to allow packaging of oracledb_exporter #308
Conversation
Looks great!
In any case, thanks for your job. |
e79a63b
to
39c37fd
Compare
Also before we contemplate merging this and releasing a major version I'd like to get your thoughts on a driver update that wouldn't require users to download the RPMs. I can write up an issue but I found that https://github.com/sijms/go-ora works without having code users to import the oracle sdk RPMs. It also is more actively used but there was a lot of the build system that is related to working with https://github.com/mattn/go-oci8. Many popular projects like https://github.com/open-telemetry/opentelemetry-collector-contrib/ and https://github.com/DataDog/datadog-agent use this library pretty effectively as to not have users manually do all the installation to use the driver and is more portable. I will create an issue for this when I have a free moment to explore/research it further but just wanted to give a heads up that I think that will be a pre-requisite for us to fully adopt this base repo in the Grafana Agent rather than a fork :) |
@@ -472,6 +472,48 @@ Everything must compile, including mattn driver for oracle. | |||
|
|||
Next build ./... in oracledb-exporter dir, or install it. | |||
|
|||
## Import into your Golang Application |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also let me know if this snippet is too long for your preference :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, it's perfect.
That's an impressive job done so far ! For the driver, I was already considering to move to https://github.com/sijms/go-ora but didn't manage to make it. So, if you have time to do it, don't hesitate and go ahead :) |
Is it ok for you to merge? |
Yep I believe so! |
Description
Refactors the code to be importable via applications such as the grafana agent.
No code can be imported via the main package. I have also validated that this code can be imported via packaging it into the grafana-agent, albeit it utilized a different driver.
https://github.com/grafana/agent
Fixes #285
Type of change
Please delete options that are not relevant.
-- it should not but let me know if the changes require a doc update!!
How Has This Been Tested?
Built the binary and ran with the refactor:
curled endpoint:
Screenshots (if appropriate):
Checklist: