Skip to content

Commit

Permalink
fix(pyroscope.receive_http): Handle metrics registry gracefully (#2302)
Browse files Browse the repository at this point in the history
This component did not handle duplciate registrations well.

This fixes #2285.
  • Loading branch information
simonswine authored Dec 19, 2024
1 parent 3195e76 commit 2b42ec4
Show file tree
Hide file tree
Showing 2 changed files with 63 additions and 7 deletions.
28 changes: 21 additions & 7 deletions internal/component/pyroscope/receive_http/receive_http.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"sync"

"github.com/gorilla/mux"
"github.com/prometheus/client_golang/prometheus"
"golang.org/x/sync/errgroup"

"github.com/grafana/alloy/internal/component"
Expand All @@ -18,6 +19,7 @@ import (
"github.com/grafana/alloy/internal/component/pyroscope/write"
"github.com/grafana/alloy/internal/featuregate"
"github.com/grafana/alloy/internal/runtime/logging/level"
"github.com/grafana/alloy/internal/util"
)

const (
Expand Down Expand Up @@ -53,16 +55,21 @@ func (a *Arguments) SetToDefault() {
}

type Component struct {
opts component.Options
server *fnet.TargetServer
appendables []pyroscope.Appendable
mut sync.Mutex
opts component.Options
server *fnet.TargetServer
uncheckedCollector *util.UncheckedCollector
appendables []pyroscope.Appendable
mut sync.Mutex
}

func New(opts component.Options, args Arguments) (*Component, error) {
uncheckedCollector := util.NewUncheckedCollector(nil)
opts.Registerer.MustRegister(uncheckedCollector)

c := &Component{
opts: opts,
appendables: args.ForwardTo,
opts: opts,
uncheckedCollector: uncheckedCollector,
appendables: args.ForwardTo,
}

if err := c.Update(args); err != nil {
Expand Down Expand Up @@ -116,7 +123,14 @@ func (c *Component) Update(args component.Arguments) error {

c.shutdownServer()

srv, err := fnet.NewTargetServer(c.opts.Logger, "pyroscope_receive_http", c.opts.Registerer, newArgs.Server)
// [server.Server] registers new metrics every time it is created. To
// avoid issues with re-registering metrics with the same name, we create a
// new registry for the server every time we create one, and pass it to an
// unchecked collector to bypass uniqueness checking.
serverRegistry := prometheus.NewRegistry()
c.uncheckedCollector.SetCollector(serverRegistry)

srv, err := fnet.NewTargetServer(c.opts.Logger, "pyroscope_receive_http", serverRegistry, newArgs.Server)
if err != nil {
return fmt.Errorf("failed to create server: %w", err)
}
Expand Down
42 changes: 42 additions & 0 deletions internal/component/pyroscope/receive_http/receive_http_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -287,3 +287,45 @@ func testOptions(t *testing.T) component.Options {
Registerer: prometheus.NewRegistry(),
}
}

// TestUpdateArgs verifies that the component can be updated with new arguments. This explictly also makes sure that the server is restarted when the server configuration changes. And there are no metric registration conflicts.
func TestUpdateArgs(t *testing.T) {
ports, err := freeport.GetFreePorts(2)
require.NoError(t, err)

forwardTo := []pyroscope.Appendable{testAppendable(nil)}

args := Arguments{
Server: &fnet.ServerConfig{
HTTP: &fnet.HTTPConfig{
ListenAddress: "localhost",
ListenPort: ports[0],
},
},
ForwardTo: forwardTo,
}

comp, err := New(testOptions(t), args)
require.NoError(t, err)

ctx, cancel := context.WithCancel(context.Background())
t.Cleanup(cancel)

go func() {
require.NoError(t, comp.Run(ctx))
}()

waitForServerReady(t, ports[0])

comp.Update(Arguments{
Server: &fnet.ServerConfig{
HTTP: &fnet.HTTPConfig{
ListenAddress: "localhost",
ListenPort: ports[1],
},
},
ForwardTo: forwardTo,
})

waitForServerReady(t, ports[1])
}

0 comments on commit 2b42ec4

Please sign in to comment.