Skip to content

Commit

Permalink
enhance: customizable config, code cleanup, middleware
Browse files Browse the repository at this point in the history
  • Loading branch information
plyr4 committed Aug 21, 2024
1 parent c22be46 commit 3aa5b8d
Show file tree
Hide file tree
Showing 19 changed files with 401 additions and 111 deletions.
17 changes: 4 additions & 13 deletions cmd/vela-server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import (
"github.com/gin-gonic/gin"
"github.com/sirupsen/logrus"
"github.com/urfave/cli/v2"
"go.opentelemetry.io/contrib/instrumentation/github.com/gin-gonic/gin/otelgin"
"golang.org/x/sync/errgroup"
"gorm.io/gorm"
"k8s.io/apimachinery/pkg/util/wait"
Expand Down Expand Up @@ -80,20 +79,13 @@ func server(c *cli.Context) error {
return err
}

// create the tracing config
tc, err := tracing.New(c)
tc, err := tracing.FromCLIContext(c)
if err != nil {
return err
}

// opt-in for telemetry
tracingMiddleware := func(ctx *gin.Context) {}
// shutdown opt-in tracing when server closes
if tc.EnableTracing {
// initialize tracing middleware
tracingMiddleware = otelgin.Middleware(c.String("tracing.service.name"),
otelgin.WithTracerProvider(tc.TracerProvider))

// shutdown tracing when server closes
defer func() {
err := tc.TracerProvider.Shutdown(context.Background())
if err != nil {
Expand Down Expand Up @@ -213,9 +205,8 @@ func server(c *cli.Context) error {
middleware.DefaultRepoEventsMask(c.Int64("default-repo-events-mask")),
middleware.DefaultRepoApproveBuild(c.String("default-repo-approve-build")),
middleware.ScheduleFrequency(c.Duration("schedule-minimum-frequency")),

// tracing middle, which is an empty handler when tracing is disabled
tracingMiddleware,
middleware.TracingClient(tc),
middleware.TracingInstrumentation(tc),
)

addr, err := url.Parse(c.String("server-addr"))
Expand Down
4 changes: 2 additions & 2 deletions database/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ func ToContext(c Setter, d Interface) {
}

// FromCLIContext creates and returns a database engine from the urfave/cli context.
func FromCLIContext(c *cli.Context, tc *tracing.Config) (Interface, error) {
func FromCLIContext(c *cli.Context, tc *tracing.Client) (Interface, error) {
logrus.Debug("creating database engine from CLI configuration")

return New(
Expand All @@ -56,6 +56,6 @@ func FromCLIContext(c *cli.Context, tc *tracing.Config) (Interface, error) {
WithLogSlowThreshold(c.Duration("database.log.slow_threshold")),
WithLogShowSQL(c.Bool("database.log.show_sql")),
WithSkipCreation(c.Bool("database.skip_creation")),
WithTracingConfig(tc),
WithTracing(tc),
)
}
2 changes: 1 addition & 1 deletion database/context_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ func TestDatabase_FromCLIContext(t *testing.T) {
// run tests
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
_, err := FromCLIContext(test.context, &tracing.Config{EnableTracing: false})
_, err := FromCLIContext(test.context, &tracing.Client{Config: tracing.Config{EnableTracing: false}})

if test.failure {
if err == nil {
Expand Down
4 changes: 2 additions & 2 deletions database/database.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ type (
// sirupsen/logrus logger used in database functions
logger *logrus.Entry
// configurations related to telemetry/tracing
tracing *tracing.Config
tracing *tracing.Client

settings.SettingsInterface
build.BuildInterface
Expand Down Expand Up @@ -243,7 +243,7 @@ func NewTest() (Interface, error) {
WithDriver("sqlite3"),
WithEncryptionKey("A1B2C3D4E5G6H7I8J9K0LMNOPQRSTUVW"),
WithSkipCreation(false),
WithTracingConfig(&tracing.Config{EnableTracing: false}),
WithTracing(&tracing.Client{Config: tracing.Config{EnableTracing: false}}),
WithLogLevel("warn"),
WithLogShowSQL(false),
WithLogSkipNotFound(true),
Expand Down
2 changes: 1 addition & 1 deletion database/database_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ func TestDatabase_New(t *testing.T) {
WithLogSlowThreshold(test.config.LogSlowThreshold),
WithEncryptionKey(test.config.EncryptionKey),
WithSkipCreation(test.config.SkipCreation),
WithTracingConfig(&tracing.Config{EnableTracing: false}),
WithTracing(&tracing.Client{Config: tracing.Config{EnableTracing: false}}),
)

if test.failure {
Expand Down
5 changes: 2 additions & 3 deletions database/opts.go
Original file line number Diff line number Diff line change
Expand Up @@ -132,10 +132,9 @@ func WithSkipCreation(skipCreation bool) EngineOpt {
}
}

// WithTracingConfig sets the shared tracing config in the database engine.
func WithTracingConfig(tracing *tracing.Config) EngineOpt {
// WithTracing sets the shared tracing config in the database engine.
func WithTracing(tracing *tracing.Client) EngineOpt {
return func(e *engine) error {
// set the tracing config
e.tracing = tracing

return nil
Expand Down
4 changes: 2 additions & 2 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -46,11 +46,11 @@ require (
go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.28.0
go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp v1.28.0
go.opentelemetry.io/otel/sdk v1.28.0
go.opentelemetry.io/otel/trace v1.28.0
go.starlark.net v0.0.0-20240314022150-ee8ed142361c
golang.org/x/crypto v0.25.0
golang.org/x/oauth2 v0.21.0
golang.org/x/sync v0.7.0
golang.org/x/time v0.5.0
gorm.io/driver/postgres v1.5.9
gorm.io/driver/sqlite v1.5.6
gorm.io/gorm v1.25.11
Expand Down Expand Up @@ -137,12 +137,12 @@ require (
github.com/xrash/smetrics v0.0.0-20240312152122-5f08fbb34913 // indirect
github.com/yuin/gopher-lua v1.1.1 // indirect
go.opentelemetry.io/otel/metric v1.28.0 // indirect
go.opentelemetry.io/otel/trace v1.28.0 // indirect
go.opentelemetry.io/proto/otlp v1.3.1 // indirect
golang.org/x/arch v0.8.0 // indirect
golang.org/x/net v0.27.0 // indirect
golang.org/x/sys v0.22.0 // indirect
golang.org/x/text v0.16.0 // indirect
golang.org/x/time v0.5.0 // indirect
google.golang.org/genproto/googleapis/api v0.0.0-20240730163845-b1a4ccb954bf // indirect
google.golang.org/genproto/googleapis/rpc v0.0.0-20240730163845-b1a4ccb954bf // indirect
google.golang.org/grpc v1.65.0 // indirect
Expand Down
30 changes: 30 additions & 0 deletions router/middleware/tracing.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
// SPDX-License-Identifier: Apache-2.0

package middleware

import (
"github.com/gin-gonic/gin"
tracingMiddleware "github.com/go-vela/server/router/middleware/tracing"
"github.com/go-vela/server/tracing"
"go.opentelemetry.io/contrib/instrumentation/github.com/gin-gonic/gin/otelgin"
)

// TracingClient is a middleware function that attaches the tracing config
// to the context of every http.Request.
func TracingClient(tc *tracing.Client) gin.HandlerFunc {
return func(c *gin.Context) {
tracingMiddleware.ToContext(c, tc)

c.Next()
}
}

// TracingInstrumentation is a middleware function that attaches the tracing config
// to the context of every http.Request.
func TracingInstrumentation(tc *tracing.Client) gin.HandlerFunc {
if tc.EnableTracing {
return otelgin.Middleware(tc.ServiceName, otelgin.WithTracerProvider(tc.TracerProvider))
}

return func(c *gin.Context) {}

Check failure on line 29 in router/middleware/tracing.go

View workflow job for this annotation

GitHub Actions / golangci

[golangci] router/middleware/tracing.go#L29

unused-parameter: parameter 'c' seems to be unused, consider removing or renaming it as _ (revive)
Raw output
router/middleware/tracing.go:29:14: unused-parameter: parameter 'c' seems to be unused, consider removing or renaming it as _ (revive)
	return func(c *gin.Context) {}
	            ^
}
37 changes: 37 additions & 0 deletions router/middleware/tracing/context.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
// SPDX-License-Identifier: Apache-2.0

package tracing

import (
"context"

"github.com/go-vela/server/tracing"
)

const key = "tracing"

// Setter defines a context that enables setting values.
type Setter interface {
Set(string, interface{})
}

// FromContext returns the associated value with this context.
func FromContext(c context.Context) *tracing.Client {
value := c.Value(key)
if value == nil {
return nil
}

tc, ok := value.(*tracing.Client)
if !ok {
return nil
}

return tc
}

// ToContext adds the value to this context if it supports
// the Setter interface.
func ToContext(c Setter, tc *tracing.Client) {
c.Set(key, tc)
}
95 changes: 95 additions & 0 deletions router/middleware/tracing/context_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
// SPDX-License-Identifier: Apache-2.0

package tracing

import (
"testing"

"github.com/gin-gonic/gin"

Check failure on line 8 in router/middleware/tracing/context_test.go

View workflow job for this annotation

GitHub Actions / golangci

[golangci] router/middleware/tracing/context_test.go#L8

File is not `gci`-ed with --skip-generated -s standard -s default -s blank -s dot -s prefix(github.com/go-vela) --custom-order (gci)
Raw output
router/middleware/tracing/context_test.go:8: File is not `gci`-ed with --skip-generated -s standard -s default -s blank -s dot -s prefix(github.com/go-vela) --custom-order (gci)
	"github.com/gin-gonic/gin"
"github.com/go-vela/server/tracing"
)

func TestTracing_FromContext(t *testing.T) {
// setup types
serviceName := "vela-test"
want := &tracing.Client{
Config: tracing.Config{
ServiceName: serviceName,
},
}

// setup context
gin.SetMode(gin.TestMode)
context, _ := gin.CreateTestContext(nil)
context.Set(key, want)

// run test
got := FromContext(context)

if got != want {
t.Errorf("FromContext is %v, want %v", got, want)
}
}

func TestTracing_FromContext_Bad(t *testing.T) {
// setup context
gin.SetMode(gin.TestMode)
context, _ := gin.CreateTestContext(nil)
context.Set(key, nil)

// run test
got := FromContext(context)

if got != nil {
t.Errorf("FromContext is %v, want nil", got)
}
}

func TestTracing_FromContext_WrongType(t *testing.T) {
// setup context
gin.SetMode(gin.TestMode)
context, _ := gin.CreateTestContext(nil)
context.Set(key, 1)

// run test
got := FromContext(context)

if got != nil {
t.Errorf("FromContext is %v, want nil", got)
}
}

func TestTracing_FromContext_Empty(t *testing.T) {
// setup context
gin.SetMode(gin.TestMode)
context, _ := gin.CreateTestContext(nil)

// run test
got := FromContext(context)

if got != nil {
t.Errorf("FromContext is %v, want nil", got)
}
}

func TestTracing_ToContext(t *testing.T) {
// setup types
serviceName := "vela-test"
want := &tracing.Client{
Config: tracing.Config{
ServiceName: serviceName,
},
}

// setup context
gin.SetMode(gin.TestMode)
context, _ := gin.CreateTestContext(nil)
ToContext(context, want)

// run test
got := context.Value(key)

if got != want {
t.Errorf("ToContext is %v, want %v", got, want)
}
}
10 changes: 10 additions & 0 deletions router/middleware/tracing/doc.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
// SPDX-License-Identifier: Apache-2.0

// Package tracing provides the ability for inserting
// or extracting the Vela OTEL tracing client
// from the middleware chain for the API.
//
// Usage:
//
// import "github.com/go-vela/server/router/middleware/tracing"
package tracing
28 changes: 28 additions & 0 deletions router/middleware/tracing/tracing.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
// SPDX-License-Identifier: Apache-2.0

package tracing

import (
"github.com/gin-gonic/gin"
"github.com/sirupsen/logrus"

"github.com/go-vela/server/tracing"
)

// Retrieve gets the value in the given context.
func Retrieve(c *gin.Context) *tracing.Client {
return FromContext(c)
}

// Establish sets the value in the given context.
func Establish() gin.HandlerFunc {
return func(c *gin.Context) {
l := c.MustGet("logger").(*logrus.Entry)
tc := Retrieve(c)

l.Debugf("reading tracing client from context")

ToContext(c, tc)
c.Next()
}
}
Loading

0 comments on commit 3aa5b8d

Please sign in to comment.