Skip to content
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

middleware: Use map for external dependencies #401

Merged
merged 3 commits into from
Feb 7, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions .github/workflows/golang-ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,11 @@ jobs:
matrix:
go: [ '1.23' ]
steps:
- uses: actions/checkout@v2
- uses: actions/setup-go@v2
- uses: actions/checkout@v4
- uses: actions/setup-go@v5
with:
go-version: ${{ matrix.go }}
check-latest: true
- name: Login to DockerHub
uses: docker/login-action@v3
with:
Expand Down
3 changes: 2 additions & 1 deletion grpc/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,8 @@ func TestPrepareContext(t *testing.T) {

externalDependencyContext := middleware.ExternalDependencyContextFromContext(ctx3)
require.NotNil(t, externalDependencyContext)
assert.Equal(t, "foo:60000,bar:1000", externalDependencyContext.String())
// Output is sorted by name
assert.Equal(t, "bar:1000,foo:60000", externalDependencyContext.String())

ctx = metadata.NewIncomingContext(context.Background(), metadata.MD{})

Expand Down
46 changes: 16 additions & 30 deletions http/middleware/external_dependency.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@ package middleware
import (
"context"
"fmt"
"maps"
"net/http"
"slices"
"strconv"
"strings"
"sync"
Expand All @@ -14,9 +16,6 @@ import (
"github.com/pace/bricks/maintenance/log"
)

// depFormat is the format of a single dependency report
const depFormat = "%s:%d"

// ExternalDependencyHeaderName name of the HTTP header that is used for reporting
const ExternalDependencyHeaderName = "External-Dependencies"

Expand Down Expand Up @@ -85,30 +84,29 @@ func ExternalDependencyContextFromContext(ctx context.Context) *ExternalDependen
// during the request livecycle
type ExternalDependencyContext struct {
mu sync.RWMutex
dependencies []externalDependency
dependencies map[string]time.Duration
}

func (c *ExternalDependencyContext) AddDependency(name string, duration time.Duration) {
c.mu.Lock()
c.dependencies = append(c.dependencies, externalDependency{
Name: name,
Duration: duration,
})
c.mu.Unlock()
defer c.mu.Unlock()

if c.dependencies == nil {
c.dependencies = make(map[string]time.Duration)
}

c.dependencies[name] += duration
}

// String formats all external dependencies
// String formats all external dependencies. The format is "name:duration[,name:duration]..." and sorted by name.
func (c *ExternalDependencyContext) String() string {
var b strings.Builder
sep := len(c.dependencies) - 1
for _, dep := range c.dependencies {
b.WriteString(dep.String())
if sep > 0 {
b.WriteByte(',')
sep--
}

for _, key := range slices.Sorted(maps.Keys(c.dependencies)) {
b.WriteString(fmt.Sprintf("%s:%d,", key, c.dependencies[key].Milliseconds()))
}
return b.String()

return strings.TrimRight(b.String(), ",")
}

// Parse a external dependency value
Expand All @@ -127,15 +125,3 @@ func (c *ExternalDependencyContext) Parse(s string) {
c.AddDependency(value[:index], time.Millisecond*time.Duration(dur))
}
}

// externalDependency represents one external dependency that
// was involved in the process to creating a response
type externalDependency struct {
Name string // canonical name of the source
Duration time.Duration // time spend with the external dependency
}

// String returns a formated single external dependency
func (r externalDependency) String() string {
return fmt.Sprintf(depFormat, r.Name, r.Duration.Milliseconds())
}
8 changes: 8 additions & 0 deletions http/middleware/external_dependency_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,10 @@ func Test_ExternalDependencyContext_String(t *testing.T) {

edc.AddDependency("test4", time.Millisecond*123)
assert.EqualValues(t, "test1:1,test2:0,test3:1000,test4:123", edc.String())

// This should update the previous value
edc.AddDependency("test4", time.Millisecond*123)
assert.EqualValues(t, "test1:1,test2:0,test3:1000,test4:246", edc.String())
}

func Test_ExternalDependencyContext_Parse(t *testing.T) {
Expand All @@ -78,4 +82,8 @@ func Test_ExternalDependencyContext_Parse(t *testing.T) {

edc.Parse("test3:1000,test4:123")
assert.EqualValues(t, "test1:1,test2:0,test3:1000,test4:123", edc.String())

// This should update the previous value
edc.Parse("test3:1000")
assert.EqualValues(t, "test1:1,test2:0,test3:2000,test4:123", edc.String())
}