Skip to content

Comments

Add Oracle DB driver in extra + fix make build for arm64#575

Closed
arnaudlemaignen wants to merge 2 commits intoburningalchemist:masterfrom
arnaudlemaignen:master
Closed

Add Oracle DB driver in extra + fix make build for arm64#575
arnaudlemaignen wants to merge 2 commits intoburningalchemist:masterfrom
arnaudlemaignen:master

Conversation

@arnaudlemaignen
Copy link

Hi burningalchemist

I know that this exporter is agnostic and can support different kind of drivers as you mentioned here
#207

Still it would be very valuable to have the Oracle DB off the shelf as it is in the TOP 5 of the DB market. It would be appealing for many people...
I have being using Corundex and Free Exporter with Oracle DB driver (godror) for more than 5 years now, and it worked pretty well.
Those exporters are not maintained so I decided to switch to yours which contains many refactoring which is all good.

I would be happy to contribute to your project and test every new release in an Oracle DB context.

Regards
Arnaud

@burningalchemist
Copy link
Owner

burningalchemist commented Oct 15, 2024

Hey @arnaudlemaignen, sorry for the silence, I was unavailable for sometime.

I think we can try to integrate ora/v2 as a part of default bundle. As mentioned in the other thread, personally I'd avoid it (to keep the binary small), given that users can create their own fork with the drivers they need.

However, if there is a way to run Oracle DB locally (and it seems to be the case afaict) and it is (is it?) a popular choice, then my former claim is no longer valid.

I'd kindly ask to split this PR in two - one for the driver, and the other for the build script, so it's easier to track.

@burningalchemist burningalchemist added the question Further information is requested label Oct 25, 2024
@burningalchemist
Copy link
Owner

@arnaudlemaignen I'll close this PR for the time being. Let me know in the comments if you want to work on it later, and I'm happy to reopen. 👍

@eirisdg
Copy link

eirisdg commented Dec 3, 2024

Hi @burningalchemist , I'm trying sql_exporter for our production servers that are in SQL/MySQL and Oracle. Of course, this development is perfect, but need Oracle support.

To not increase the binary, an option could be create 2 differents containers, one without oracle and another one with oracle support. Could it be?

@msavdert
Copy link

msavdert commented Dec 3, 2024

Hi @burningalchemist,
I'm using sql_exporter for our production servers. Oracle support would be a fantastic addition.
Thank you!

@burningalchemist
Copy link
Owner

burningalchemist commented Dec 4, 2024

Hey @eirisdg, you may be fine with a single container, which contains only the drivers you use, e.g. MySQL and Oracle. For that you'd need to adjust drivers.go directly or for safety, adjust drivers_gen.go: just add those drivers to the custom list, and then run make drivers-custom. In this case, you can always regenerate the drivers.go with different templates. Overall, the resulting binary will be pretty small, since we don't embed vertica, snowflake, mssql, Clickhouse, and PostgreSQL drivers.

(the implication is that you have golang installed on your machine, though it's pretty easy to do these days)

@burningalchemist
Copy link
Owner

Hey @msavdert, thanks for your feedback. 👍 Yeah, I'll try to include it to the next release then, and let's check the binary size.

@eirisdg
Copy link

eirisdg commented Dec 4, 2024

Hey @burningalchemist.

I just wanted to avoid creating a new fork of the repository to add Oracle support, especially considering that your fork is the one currently maintained and linked on the Prometheus website.

So, if support is included in this fork, it would be ideal.

Thanks!

@aelogonpin
Copy link

Hey @burningalchemist.

I just wanted to avoid creating a new fork of the repository to add Oracle support, especially considering that your fork is the one currently maintained and linked on the Prometheus website.

So, if support is included in this fork, it would be ideal.

Thanks!

I agree with @eirisdg. As the "extra-official" fork linked on the Prometheus website, I believe it should include support if possible. It would be very useful for all of us who need it.

@burningalchemist
Copy link
Owner

burningalchemist commented Dec 4, 2024

Hey @eirisdg, @aelogonpin

Oftentimes companies fork the open repos into the internal organization for security measures, audit and stuff. And so it gives them flexibility of configiration, also the security department is happy. For sql_exporter it's literally a oneliner to bring your own driver, but I agree that it comes with a price of watching the upstream repo for bugfixes, rebasing and the other git fun. 😅

As I said, I'll try to include the driver into the upcoming release. 👍

@eirisdg
Copy link

eirisdg commented Dec 4, 2024

@burningalchemist I like reading that, thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

question Further information is requested

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants