Skip to content

Commit

Permalink
Use confighttp in expvar extension (#6227)
Browse files Browse the repository at this point in the history
## Which problem is this PR solving?
- expvar extension configuration was using non-standard config that did
not allow specifying host to listen to

## Description of the changes
- Change it to use OTEL helper

## How was this change tested?
- `go run ./cmd/jaeger`
- `curl http://localhost:27777`

---------

Signed-off-by: Yuri Shkuro <[email protected]>
  • Loading branch information
yurishkuro authored Nov 21, 2024
1 parent 48cf1ba commit f43870b
Show file tree
Hide file tree
Showing 5 changed files with 50 additions and 13 deletions.
2 changes: 1 addition & 1 deletion cmd/jaeger/internal/all-in-one.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ extensions:
grpc:

expvar:
port: 27777
endpoint: "${env:JAEGER_LISTEN_HOST:-localhost}:27777"

receivers:
otlp:
Expand Down
3 changes: 2 additions & 1 deletion cmd/jaeger/internal/extension/expvar/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,11 @@ package expvar

import (
"github.com/asaskevich/govalidator"
"go.opentelemetry.io/collector/config/confighttp"
)

type Config struct {
Port int `mapstructure:"port"`
confighttp.ServerConfig `mapstructure:",squash"`
}

func (cfg *Config) Validate() error {
Expand Down
28 changes: 19 additions & 9 deletions cmd/jaeger/internal/extension/expvar/extension.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,9 @@ import (
"errors"
"expvar"
"fmt"
"net"
"net/http"
"time"
"sync"

"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/component/componentstatus"
Expand All @@ -27,6 +28,8 @@ type expvarExtension struct {
config *Config
server *http.Server
telset component.TelemetrySettings

shutdownWG sync.WaitGroup
}

func newExtension(config *Config, telset component.TelemetrySettings) *expvarExtension {
Expand All @@ -36,20 +39,26 @@ func newExtension(config *Config, telset component.TelemetrySettings) *expvarExt
}
}

func (c *expvarExtension) Start(_ context.Context, host component.Host) error {
c.server = &http.Server{
Addr: fmt.Sprintf(":%d", c.config.Port),
Handler: expvar.Handler(),
ReadHeaderTimeout: 3 * time.Second,
func (c *expvarExtension) Start(ctx context.Context, host component.Host) error {
server, err := c.config.ToServer(ctx, host, c.telset, expvar.Handler())
if err != nil {
return err
}
c.server = server
var hln net.Listener
if hln, err = c.config.ToListener(ctx); err != nil {
return err
}
c.telset.Logger.Info("Starting expvar server", zap.String("addr", c.server.Addr))
c.telset.Logger.Info("Starting expvar server", zap.Stringer("addr", hln.Addr()))
c.shutdownWG.Add(1)
go func() {
if err := c.server.ListenAndServe(); err != nil && !errors.Is(err, http.ErrServerClosed) {
defer c.shutdownWG.Done()

if err := c.server.Serve(hln); err != nil && !errors.Is(err, http.ErrServerClosed) {
err = fmt.Errorf("error starting expvar server: %w", err)
componentstatus.ReportStatus(host, componentstatus.NewFatalErrorEvent(err))
}
}()

return nil
}

Expand All @@ -58,6 +67,7 @@ func (c *expvarExtension) Shutdown(ctx context.Context) error {
if err := c.server.Shutdown(ctx); err != nil {
return fmt.Errorf("error shutting down expvar server: %w", err)
}
c.shutdownWG.Wait()
}
return nil
}
24 changes: 23 additions & 1 deletion cmd/jaeger/internal/extension/expvar/extension_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ import (
"github.com/open-telemetry/opentelemetry-collector-contrib/extension/storage/storagetest"
"github.com/stretchr/testify/require"
"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/config/configauth"
"go.opentelemetry.io/collector/config/confighttp"
"go.uber.org/zap/zaptest"
)

Expand All @@ -30,7 +32,9 @@ func TestExpvarExtension(t *testing.T) {
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
config := &Config{
Port: Port,
ServerConfig: confighttp.ServerConfig{
Endpoint: "0.0.0.0:27777",
},
}
s := newExtension(config, component.TelemetrySettings{
Logger: zaptest.NewLogger(t),
Expand All @@ -51,3 +55,21 @@ func TestExpvarExtension(t *testing.T) {
})
}
}

func TestExpvarExtension_StartError(t *testing.T) {
config := &Config{
ServerConfig: confighttp.ServerConfig{
Endpoint: "0.0.0.0:27777",
Auth: &confighttp.AuthConfig{
Authentication: configauth.Authentication{
AuthenticatorID: component.MustNewID("invalid_auth"),
},
},
},
}
s := newExtension(config, component.TelemetrySettings{
Logger: zaptest.NewLogger(t),
})
err := s.Start(context.Background(), storagetest.NewStorageHost())
require.ErrorContains(t, err, "invalid_auth")
}
6 changes: 5 additions & 1 deletion cmd/jaeger/internal/extension/expvar/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,10 @@ package expvar

import (
"context"
"fmt"

"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/config/confighttp"
"go.opentelemetry.io/collector/extension"
)

Expand All @@ -27,7 +29,9 @@ func NewFactory() extension.Factory {

func createDefaultConfig() component.Config {
return &Config{
Port: Port,
ServerConfig: confighttp.ServerConfig{
Endpoint: fmt.Sprintf("0.0.0.0:%d", Port),
},
}
}

Expand Down

0 comments on commit f43870b

Please sign in to comment.