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

RFE: Multi-Account? Or at least track authenticated and unauthenticated #5

Open
TJM opened this issue Dec 8, 2020 · 3 comments
Open

Comments

@TJM
Copy link
Contributor

TJM commented Dec 8, 2020

How much effort would it be to track both authenticated and unauthenticated requests using the same exporter. We might have to change the name of the metric slightly, but I think it would be better to do that as one exporter rather than two separate ones.

Are most people are probably using a "paid" account, where their requests are just unlimited? I would have to assume if someone is tracking this statistic, they are not using a paid account? right?

~tommy

@mstein11
Copy link
Contributor

mstein11 commented Dec 9, 2020

Shouldn't be too much effort. My first thought was to implement that with multiple instances of the exporter. But it seems you would prefer to add this capability to the script? What's your reasoning behind that? Is it because of the increased resource consumption for having multiple pods running the script or do you have other reasons? I think, if we tweak the resource requests and limits a little, we might be able to reasonably run multiple instances of the exporter to achieve that, but I am also open to add that capability to the python script.

What would be the use case for that? I am not sure if somebody would configure their infrastructure to sometimes use authenticated requests and sometimes not, but I might be wrong here.

@TJM
Copy link
Contributor Author

TJM commented Dec 9, 2020

I was afraid the with multiple instances of the exporter, the stats would get mixed up in prometheus, having a single instance would allow it to monitor authenticated and unauthenticated requests and keep the stats separate, but I am honestly a prometheus n00b :). If we have multiple instances, we will for sure need to be a way to define the statistic parameters?

USE CASE: We have a docker personal access token configured on our "artifactory" remote docker repo, and I would like to track its usage, as I think it is limited to 200 pulls. Additionally I would also like to track the "unauthenticated" pulls coming from kube worker nodes, for people that have not pointed their images to artifactory yet.

EDIT: Also while in there, the refresh interval of 10s seems excessive, probably should be 30 and/or configurable.

@mstein11
Copy link
Contributor

mstein11 commented Dec 11, 2020

Hey @TJM ,
I created a small proof of concept for the multi account feature. I pushed it to the poc/multiple-dockerhub-accounts branch. Mind taking a look at it and evaluating if it solves your use case?

I implemented it through helm by deploying one instance of the exporter for each entry under configs in the helm values file.

Prometheus can distinguish between the different exporter metrics by the attached metadata of each metric (?), see the attached screenshot. Unfortunately, the existing grafana dashboard is not able to handle more the one stream of the same metric with differing metadata, if we want to go down this road we would need to tweak the dashboard a little, this shouldn't be a big problem however.

I am still open to implement this feature through changes in the script itself and with only one deployment, I think both approaches have benefits to it.

Pro multiple deployments:

  • its easy to implement a dynamic number of exporters for multiple authentication contexts (unauthenticated plus multiple authenticated accounts)
  • it's more simple because we don't need to tweak the script itself, thus reducing complexity if one is facing the simplest use case which is just one authentication context

Pro implementing the feature in the python script

  • consumes less resources when having multiple authentication requests since only one deployment is created
  • It doesn't break the current api. For the above mentioned method, we need to have a new structure of the value file. The config block is replaced by a configs block which is an array instead of a map. This can break existing installations since most people probably have implemented it through a CI pipeline which only pulls the main branch of this repo while having a values file somewhere which would be incompatible with the new feature. So we need to find a solution for that if we go with the existing poc.

What's your opinion on that?

P.S.: mind opening a new issue for changing the refresh interval so we can keep things seperate?
Bildschirmfoto 2020-12-11 um 13 51 52

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

No branches or pull requests

2 participants