@xperimental is our Marathon expert. Robert, would you be able to have a look at this?

Thanks.

I assume you could just as well use bearer tokens etc. – should we unify the auth approaches we want to support (i.e. same ones as for scraping) into an object we just provide with every network-based SD?

@brian-brazil

should we unify the auth approaches we want to support (i.e. same ones as for scraping) into an object we just provide with every network-based SD?

Yes, we need it in the AM and blackbox exporter too so this should ultimately go into common.

Again, sorry for the delay. Tested it with a local Marathon installation, the implementation here works fine.

That said, there is an alternative already, because the credentials could always be provided using the URL like this (the disadvantage being, that the password is clearly readable on the configuration page):

scrape_configs:
- job_name: local-marathon
  marathon_sd_configs:
  - servers:
    - http://user:password@marathon.server.local:8080

Currently the open-source version of Marathon only supports Basic-Authentication, so it would probably not benefit from a "generic" implementation. I can not speak for the commercial version though.

I also noticed that the error message is says pretty much nothing about the actual problem when you have a Marathon server with authentication running and prometheus not configured with the correct credentials:

ERRO[0005] Error while updating services: invalid character '<' looking for beginning of value  source=marathon.go:67

This has already been the case before, so it would probably be good to fix this even if this PR does not make it. A simple check for the HTTP status code should be enough.

Are there any plans to merge this finally?
Having basic auth for SD with marathon would be great.
Having my credentials in the server url and thus readable for everyone with access to prometheus is not really an option.

@ChillkroeteTTS To be honest I lost track of this issue and don't know what the decision on "should this be merged" was, if there was any.

By now the PR is also a bit outdated and needs rebasing. I had hoped that there was a way to specify the credentials by now when I read your comment the first time, but this sadly does not seem to be the case, so maybe this PR should be revived.

@ChillkroeteTTS @xperimental @gondor I'm happy to review an updated PR.

Using HTTPClientConfig would be better than just doing basic auth.

This still needs to be updated to use our usual http client config code. Feel free to send a new PR with these changes, this one is rather stale by now.