Skip to content

[Prometheus Receiver] Add in setting to enable server for Prometheus UI #29622

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

Closed
wants to merge 2 commits into from

Conversation

gracewehner
Copy link
Contributor

@gracewehner gracewehner commented Dec 2, 2023

Description:

Adds a feature to the Prometheus Receiver to allow enabling the server for the Prometheus UI using the Prometheus web package. The React app from the Prometheus repo will need to be built and run alongside the collector to actually use the server. However, there are discussions to publish this UI to make it easier to use outside of the Prometheus binary.

The UI allows easier debugging of scrape configs, target discovery, and error messages for failed scrapes. The UI is set to run in agent mode so the UI will not have the graph for querying, but will have the /config, /service-discovery, and /targets pages.

image image

image

image

Link to tracking Issue: open-telemetry/prometheus-interoperability-spec#63

Testing:
Added unit tests to ensure the server is only enabled when the setting is added in the config and to make sure the changes are backwards compatible. Tested running the otelcollector along with the React app that the UI is accessible and displays the correct information. Tested with and without the target allocator.

Documentation:
Added to the Prometheus Receiver README instructions for how to use.

@gracewehner gracewehner requested a review from a team December 2, 2023 00:01
@github-actions github-actions bot added the receiver/prometheus Prometheus receiver label Dec 2, 2023
Copy link
Contributor

@dashpole dashpole left a comment

Choose a reason for hiding this comment

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

Overall, i'm hesitant to add this functionality to a receiver, but I understand how useful it can be. Is there a way to add this as an extension, similar to the remote tap extension? https://github.com/open-telemetry/opentelemetry-collector-contrib/tree/main/extension/remotetapextension

webOptions := web.Options{
ScrapeManager: r.scrapeManager,
Context: ctx,
ListenAddress: fmt.Sprintf(":%d", prometheusUIServerPort),
Copy link
Contributor

Choose a reason for hiding this comment

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

This means only a single prometheus receiver can have the web UI enabled at once, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes that's right, per VM or pod it's running on. I have added a setting to make this port configurable

@@ -497,6 +497,10 @@ func TestTargetAllocatorJobRetrieval(t *testing.T) {

require.NoError(t, receiver.Start(ctx, componenttest.NewNopHost()))

t.Cleanup(func() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this related to the change, or just a useful fix?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is from running all the tests with the UI enabled by default. Some tests are not cleaning up before the next one runs so the port is already taken on the next test. I could also remove these changes since it's not enabled by default and has separate tests for the UI

@@ -17,13 +17,16 @@ import (
"time"

"github.com/go-kit/log"
"github.com/google/cadvisor/version"
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this the right import? Seems odd to depend on cAdvisor...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right, this should be the Prometheus version package, fixed this

@gracewehner
Copy link
Contributor Author

Overall, i'm hesitant to add this functionality to a receiver, but I understand how useful it can be. Is there a way to add this as an extension, similar to the remote tap extension? https://github.com/open-telemetry/opentelemetry-collector-contrib/tree/main/extension/remotetapextension

Thanks @dashpole, after reading more about the extensions design, I see why it would fit more as an extension. I looked into the remote tap extension and created a similar extension for the UI to see if this would work. The UI needs 2 things from the prom receiver to get all the data correctly:

  1. the Prom config
  2. the scrape manager

The remotetapextension is getting the data from the remotetapprocessor. The implementation hasn't been added yet for these to the repo, but I looked a bit more into it after this morning and found this as part of the POC example that fixes having to export the whole extension type for access from the receiver: https://github.com/pmcollins/opentelemetry-collector-contrib/blob/9dfc2cb971b6181beb919038fc715c06f123296b/processor/websocketprocessor/processor.go#L88

So at the end of the Start() function of the Prom Receiver the following can be added to pass the config and scrapemanager to the extension:

	extensions := host.GetExtensions()
	for k, v := range extensions {
		if k.Type() == "prometheusui" {
			ext := v.(interface{ RegisterPrometheusReceiverComponents(*config.Config, *scrape.Manager) })
			ext.RegisterPrometheusReceiverComponents(baseCfg, r.scrapeManager)
		}
	}

I can make this change in this PR so that this is the only functionality added in the Prom Receiver. If we don't want this to always run, I can also keep the enable_prometheus_ui setting to go around this code? Then I can open a PR also for the extension.

(As reference, here's what the extension would look like:)


package prometheusuiextension // import "github.com/open-telemetry/opentelemetry-collector-contrib/extension/prometheusuiextension"

import (
	"context"
	"fmt"
	"io"
	"net/url"
	"time"

	"github.com/go-kit/kit/log"

	"github.com/prometheus/client_golang/prometheus"
	"github.com/prometheus/common/version"
	"github.com/prometheus/prometheus/config"
	"github.com/prometheus/prometheus/scrape"
	"github.com/prometheus/prometheus/web"
	"go.opentelemetry.io/collector/component"
	"go.opentelemetry.io/collector/extension"
	"go.uber.org/zap"
)

type prometheusUIExtension struct {
	config   				   *Config
	settings 				   extension.CreateSettings
	prometheusConfig   *config.Config
	scrapeManager      *scrape.Manager
	webHandler         *web.Handler
	cancelFunc         context.CancelFunc
	ctx                context.Context
}

  // Use same settings as Prometheus web server
const (
	maxConnections     			= 512
	readTimeoutMinutes 			= 10
	prometheusUIServerPort 	= 9090
)

func (e *prometheusUIExtension) Start(_ context.Context, host component.Host) error {
	e.ctx, e.cancelFunc = context.WithCancel(context.Background())

	fmt.Println("starting prometheusuiextension")

	return nil

}

func (e *prometheusUIExtension) RegisterPrometheusReceiverComponents(config *config.Config, scrapeManager *scrape.Manager) {
	e.prometheusConfig = config
	e.scrapeManager = scrapeManager
	e.startWebHandler()
}

func (e *prometheusUIExtension) startWebHandler() error {
	fmt.Println("prometheusuiextension ready")

	webOptions := web.Options{
		ScrapeManager: e.scrapeManager,
		Context:       e.ctx,
		ListenAddress: fmt.Sprintf(":%d", prometheusUIServerPort),
		ExternalURL: &url.URL{
			Scheme: "http",
			Host:   fmt.Sprintf("localhost:%d", prometheusUIServerPort),
			Path:   "",
		},
		RoutePrefix: "/",
		ReadTimeout: time.Minute * readTimeoutMinutes,
		PageTitle:   "Prometheus Receiver",
		Version: &web.PrometheusVersion{
			Version:   version.Version,
			Revision:  version.Revision,
			Branch:    version.Branch,
			BuildUser: version.BuildUser,
			BuildDate: version.BuildDate,
			GoVersion: version.GoVersion,
		},
		Flags:          make(map[string]string),
		MaxConnections: maxConnections,
		IsAgent:        true,
		Gatherer:       prometheus.DefaultGatherer,
	}
	go_kit_logger := log.NewLogfmtLogger(log.NewSyncWriter(io.Discard))
	e.webHandler = web.New(go_kit_logger, &webOptions)

	listener, err := e.webHandler.Listener()
	if err != nil {
		return err
	}
	// Pass the config config to the web handler and signify its ready
	// since Prometheus allows reloading the config without restarting.
	e.webHandler.ApplyConfig(e.prometheusConfig)
	e.webHandler.SetReady(true)

	// Use the same context as the discovery and scrape managers for shutting down.
	// The webHandler handles shutting down the server gracefully.
	go func() {
		if err := e.webHandler.Run(e.ctx, listener, ""); err != nil {
			e.settings.Logger.Error("Web handler failed", zap.Error(err))
			return
		}
	}()

	return nil
}

func (e *prometheusUIExtension) Shutdown(_ context.Context) error {
	if e.cancelFunc != nil {
		e.cancelFunc()
	}

	fmt.Println("shutting down prometheusuiextension")
	return nil
}

@dashpole
Copy link
Contributor

dashpole commented Dec 7, 2023

Thanks @gracewehner for the investigation. The approach you laid out makes sense to me. A follow-up  question:

@gracewehner do you think we can pass the prom receiver name in to RegisterPrometheusReceiverComponents as well? That would, in theory, allow us to support multiple prometheus receivers.

One way to do this (I think?) would be for each receiver to get its own webserver at a different route. Maybe they could each listen at Route:/<prometheus_receiver_name>/? If you don't name the receiver, it would still get Route: "/".

I.e. the UI for:

receivers:
    prometheus:

would be at route "/". The UI for:

receivers:
    prometheus/foobar:

would be at route: /foobar/

@dashpole
Copy link
Contributor

dashpole commented Dec 7, 2023

@atoulme, since you are working on the remote tap extension

  1. Do you think serving the prometheus UI also belongs in its own extension? Do you think it matters that this extension would be specific to the prometheus receiver?
  2. Is the remotetapextension able to handle multiple remotetapprocessors?
  3. Does the remotetapextension bundle any UI artifacts? If so, how does it do that?

Thanks!

@atoulme
Copy link
Contributor

atoulme commented Dec 7, 2023

  1. I'd make its own extension just since it might have enough settings that would derail the prometheus receiver configuration.
  2. yes, though it's fiction still at this point
  3. it embeds js and html using go:embed

@dashpole
Copy link
Contributor

dashpole commented Dec 7, 2023

Thanks @atoulme. @gracewehner, lets move forward with the changes to the prometheus receiver here, and open an issue for a new component for the prometheusuiextension. I can sponsor, and we can both be codeowners if that is alright with you.

@Aneurysm9
Copy link
Member

I'm a bit wary of including the entire Prometheus API when we can only support a small subset of its features. I have a PR I can resurrect that implements only the /targets resource in a way that will allow multiple receivers to expose the API on independent ports. I'm not sure that attempting to consolidate multiple receivers on the same port through an extension will be good or valuable here as the API wouldn't provide a means of differentiating between the receiver configurations. There would be no way to tell which receiver a given job/target relate to.

@dashpole
Copy link
Contributor

I'm not sure that attempting to consolidate multiple receivers on the same port through an extension will be good or valuable here as the API wouldn't provide a means of differentiating between the receiver configurations.

Do you think having them on different routes is enough? Or do you think different ports is better?

@dashpole
Copy link
Contributor

@Aneurysm9 having the UI also seems more useful than just serving the /targets endpoint. Is it possible to use the UI with your PR?

@gracewehner
Copy link
Contributor Author

I'm a bit wary of including the entire Prometheus API when we can only support a small subset of its features. I have a PR I can resurrect that implements only the /targets resource in a way that will allow multiple receivers to expose the API on independent ports. I'm not sure that attempting to consolidate multiple receivers on the same port through an extension will be good or valuable here as the API wouldn't provide a means of differentiating between the receiver configurations. There would be no way to tell which receiver a given job/target relate to.

When creating the webhandler in agent mode, only the /configs, /service-discovery, /targets APIs are exposed which would be useful and can be supported by the receiver (plus some build info)
image

Do you think having them on different routes is enough? Or do you think different ports is better?

To add to this, from looking at the way the prometheus web handler is setup, a lot of changes would need to be made on the Prometheus-side to be able to support different routes pointing to different scrapemanagers at the same port. Using different ports would be supported though

@dashpole
Copy link
Contributor

I do agree that trying to merge receivers would make the UI confusing. We could add the configuration to the prometheus receiver for the UI port. It would be nice to also somehow detect conflicts.

@Aneurysm9
Copy link
Member

@Aneurysm9 having the UI also seems more useful than just serving the /targets endpoint. Is it possible to use the UI with your PR?

Having the UI means we're taking on an entire front-end application into our footprint. I certainly don't have the expertise to maintain it or even really to understand the issues that may present or how to deal with security patching and maintenance. My understanding is that the front-end application gets its information from the API, so as long as we're exposing a compatible API it should be possible to point the UI at it and have it work.

@Aneurysm9
Copy link
Member

When creating the webhandler in agent mode, only the /configs, /service-discovery, /targets APIs are exposed which would be useful and can be supported by the receiver (plus some build info)

/service-discovery uses the /targets API. We should have the information required for /configs if desired.

@Aneurysm9
Copy link
Member

Aneurysm9 commented Dec 11, 2023

I think that adding the entire Prometheus web UI here (even the agent-mode subset) is a significant one-way door to walk through. I'd be much more comfortable adding a /targets API implementation that can be called from a separately hosted UI instance (or any other mechanism, since it's a simple JSON resource). We can always add more if we decide that's not enough.

@gracewehner
Copy link
Contributor Author

Ok I see, thanks @Aneurysm9, just to expand on it a bit more, the webhandler is just a wrapper around the API that then serves the react app files. The Prometheus project embeds the react app at build time, but I was thinking we can just have the webhandler and its APIs exposed and the react app files/build can be managed by the user if we don't want to directly put it in the project. Updates for the webhandler package would be the same Prometheus package updates. Then, there is an equivalent version of the react app published in each Prometheus release that can be pulled, unzipped, and put in the same directory that the otelcollector is run from: https://github.com/prometheus/prometheus/releases/tag/v2.48.1 has the ui build: https://github.com/prometheus/prometheus/releases/download/v2.48.1/prometheus-web-ui-2.48.1.tar.gz. The other idea was the build for the extension package can pull this UI and embed these files directly.

You are right that the UI is just using the same APIs. The webhandler has both /targets that exposes the UI and /api/v1/targets to expose the API. The upside with the webhandler is that these are all set up using the Prometheus web package without needing to copy over and maintain the Prometheus code for both the UI and the APIs.

The UI however can definitely be pointed to directly exposed APIs by setting up a separate instance re-using the inner web package code such as https://github.com/prometheus/prometheus/blob/main/web/web.go#L391-L443. This part could also be the extension in the future if we wanted.

@Aneurysm9
Copy link
Member

The UI however can definitely be pointed to directly exposed APIs by setting up a separate instance re-using the inner web package code such as https://github.com/prometheus/prometheus/blob/main/web/web.go#L391-L443. This part could also be the extension in the future if we wanted.

With this being possible I feel strongly that the receiver should only expose the /api/v1/targets resource in a manner that does not expose the entire Prometheus web or API handlers, such as was done in #23244. It is the smallest useful step forward that does not open us up to maintaining JS/TypeScript/React dependencies or any security vulnerabilities they or the rest of the upstream API handler may have.

I'm not sure that I like the idea of serving the web UI through an extension. It feels beyond the scope of the collector to include what is basically a static HTTP server for a specific application, especially if we're not embedding the resources it would serve. If the end user needs to bring their own assets they can also bring their own web server. We don't need to be all things to all people. That said, as long as it is completely decoupled from the receiver such that I do not need to include it in my downstream distribution, I don't think that's a hill I need to die on.

@gracewehner
Copy link
Contributor Author

Ok thanks @Aneurysm9, it would be great to open back up your PR then. I am happy to help with anything or can build upon your PR if needed

@Aneurysm9
Copy link
Member

Ok thanks @Aneurysm9, it would be great to open back up your PR then. I am happy to help with anything or can build upon your PR if needed

If we're good with that path I'll get my PR re-opened and up-to-date, probably tomorrow or Thursday. @dashpole?

@dashpole
Copy link
Contributor

I'm ok with that plan, although it does seem likely to diverge from upstream over time. If we go this route, can we also try and get prometheus to provide a web entrypoint for agents like us so we don't need to maintain a copy? It would also be good to keep copied code in separate files/directory.

Copy link
Contributor

This PR was marked stale due to lack of activity. It will be closed in 14 days.

@github-actions github-actions bot added the Stale label Dec 28, 2023
Copy link
Contributor

Closed as inactive. Feel free to reopen if this PR is still being worked on.

@github-actions github-actions bot closed this Jan 12, 2024
@jatinmehrotra
Copy link

This feature Is still needed any update on this?

I am still looking for a way to enable Prometheus WebUI for Prometheus reciever。

@gracewehner
Copy link
Contributor Author

Hi @jatinmehrotra, currently this is dependent on this open PR for the API behind the UI: #23244 (@Aneurysm9)

@jatinmehrotra
Copy link

@gracewehner Thank you for your confirmation.

Not directly related but until #23244 is merged is there a way where I can get similar metrics as exposed by prometheus server( when we configure job called prometheus) for example prometheus_build_info using OTEL collector?

I am not looking for the same metrics as I am aware this is the exposed by prometheus server but is there a way to expose metrics related to Otel collector?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
receiver/prometheus Prometheus receiver Stale
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants