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

feat: replace datadog-specific otel lib with generic otel #818

Merged
merged 3 commits into from
Dec 10, 2024
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
47 changes: 24 additions & 23 deletions .github/workflows/ci-evm.yaml
Original file line number Diff line number Diff line change
@@ -1,25 +1,26 @@
name: CI
# Temporarily disabled
# name: CI

on:
pull_request:
paths:
- evm/**
push:
paths:
- evm/**
branches:
- main
# on:
# pull_request:
# paths:
# - evm/**
# push:
# paths:
# - evm/**
# branches:
# - main

jobs:
build-evm:
name: Build (EVM)
runs-on: namespace-profile-linux-8vcpu-16gb-cached
steps:
- name: Checkout
uses: actions/checkout@v4
- name: Install Foundry
uses: foundry-rs/foundry-toolchain@v1
- name: Set up Docker Buildx
uses: docker/setup-buildx-action@v3
- name: Build Rollup
run: make rollup-build
# jobs:
# build-evm:
# name: Build (EVM)
# runs-on: namespace-profile-linux-8vcpu-16gb-cached
# steps:
# - name: Checkout
# uses: actions/checkout@v4
# - name: Install Foundry
# uses: foundry-rs/foundry-toolchain@v1
# - name: Set up Docker Buildx
# uses: docker/setup-buildx-action@v3
# - name: Build Rollup
# run: make rollup-build
2 changes: 1 addition & 1 deletion .github/workflows/ci-go.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ jobs:
- name: golangci-lint
uses: golangci/golangci-lint-action@v4
with:
version: v1.57.1
version: v1.62.2
args: --timeout=10m -v ${{ steps.go-dir.outputs.path }}
## skip cache, use Namespace volume cache
skip-cache: true
Expand Down
13 changes: 8 additions & 5 deletions .golangci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ linters-settings:
# Default: true
skipRecvDeref: false

gomnd:
mnd:
# List of function patterns to exclude from analysis.
# Values always ignored: `time.Date`,
# `strconv.FormatInt`, `strconv.FormatUint`, `strconv.FormatFloat`,
Expand Down Expand Up @@ -139,7 +139,7 @@ linters-settings:
nolintlint:
# Exclude following linters from requiring an explanation.
# Default: []
allow-no-explanation: [ funlen, gocognit, lll, exhaustruct, decorder, gomnd ]
allow-no-explanation: [funlen, gocognit, lll, exhaustruct, decorder, mnd]
# Enable to require an explanation of nonzero length after each nolint directive.
# Default: false
require-explanation: true
Expand Down Expand Up @@ -196,6 +196,10 @@ linters-settings:
# Default: false
custom-order: true

gosec:
excludes:
- G115 # Selectively disable G115 for cases where we've verified the conversions are safe

linters:
disable-all: true
enable:
Expand All @@ -217,9 +221,7 @@ linters:
- durationcheck # checks for two durations multiplied together
- errname # checks that sentinel errors are prefixed with the Err and error types are suffixed with the Error
- errorlint # finds code that will cause problems with the error wrapping scheme introduced in Go 1.13
- execinquery # checks query string in Query function which reads your Go src files and warning it finds
- exhaustive # checks exhaustiveness of enum switch statements
- exportloopref # checks for pointers to enclosing loop variables
- forbidigo # forbids identifiers
- funlen # tool for detection of long functions
- gocheckcompilerdirectives # validates go compiler directive comments (//go:)
Expand All @@ -232,7 +234,7 @@ linters:
- gocyclo # computes and checks the cyclomatic complexity of functions
#- godot # checks if comments end in a period
- goimports # in addition to fixing imports, goimports also formats your code in the same style as gofmt
- gomnd # detects magic numbers
- mnd # detects magic numbers
- gomoddirectives # manages the use of 'replace', 'retract', and 'excludes' directives in go.mod
#- gomodguard # allow and block lists linter for direct Go module dependencies. This is different from depguard where there are different block types for example version constraints and module recommendations
- goprintffuncname # checks that printf-like functions are named with f at the end
Expand Down Expand Up @@ -324,6 +326,7 @@ issues:
- noctx
- wrapcheck
- perfsprint
- errcheck
# TODO: remove after PR is released https://github.com/golangci/golangci-lint/pull/4386
- text: "fmt.Sprintf can be replaced with string addition"
linters: [ perfsprint ]
2 changes: 1 addition & 1 deletion assert/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ require (

require (
github.com/kr/pretty v0.3.1 // indirect
github.com/rogpeppe/go-internal v1.12.0 // indirect
github.com/rogpeppe/go-internal v1.13.1 // indirect
gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c // indirect
)

Expand Down
4 changes: 2 additions & 2 deletions assert/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ github.com/pkg/diff v0.0.0-20210226163009-20ebb0f2a09e/go.mod h1:pJLUxLENpZxwdsK
github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 h1:Jamvg5psRIccs7FGNTlIRMkT8wgtp5eCXdBlqhYGL6U=
github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
github.com/rogpeppe/go-internal v1.9.0/go.mod h1:WtVeX8xhTBvf0smdhujwtBcq4Qrzq/fJaraNFVN+nFs=
github.com/rogpeppe/go-internal v1.12.0 h1:exVL4IDcn6na9z1rAb56Vxr+CgyK3nn3O+epU5NdKM8=
github.com/rogpeppe/go-internal v1.12.0/go.mod h1:E+RYuTGaKKdloAfM02xzb0FW3Paa99yedzYV+kq4uf4=
github.com/rogpeppe/go-internal v1.13.1 h1:KvO1DLK/DRN07sQ1LQKScxyZJuNnedQ5/wKSR38lUII=
github.com/rogpeppe/go-internal v1.13.1/go.mod h1:uMEvuHeurkdAXX61udpOXGD/AzZDWNMNyH2VO9fmH0o=
github.com/rotisserie/eris v0.5.4 h1:Il6IvLdAapsMhvuOahHWiBnl1G++Q0/L5UIkI5mARSk=
github.com/rotisserie/eris v0.5.4/go.mod h1:Z/kgYTJiJtocxCbFfvRmO+QejApzG6zpyky9G1A4g9s=
github.com/stretchr/testify v1.9.0 h1:HtqpIVDClZ4nwg75+f6Lvsy/wHu+3BoSGCbBAcpTsTg=
Expand Down
4 changes: 0 additions & 4 deletions cardinal/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ var (
BaseShardSequencerAddress: DefaultBaseShardSequencerAddress,
BaseShardRouterKey: "",
TelemetryTraceEnabled: false,
TelemetryProfilerEnabled: false,
}
)

Expand Down Expand Up @@ -78,9 +77,6 @@ type WorldConfig struct {

// TelemetryTraceEnabled When true, Cardinal will collect OpenTelemetry traces
TelemetryTraceEnabled bool `mapstructure:"TELEMETRY_TRACE_ENABLED"`

// TelemetryProfilerEnabled When true, Cardinal will run Datadog continuous profiling
TelemetryProfilerEnabled bool `mapstructure:"TELEMETRY_PROFILER_ENABLED"`
}

func loadWorldConfig() (*WorldConfig, error) {
Expand Down
9 changes: 4 additions & 5 deletions cardinal/filter/filter.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,14 @@ type ComponentFilter interface {
MatchesComponents(components []types.Component) bool
}

// ComponentWrapper wraps a Component type for filtering purposes.
type ComponentWrapper struct {
Component types.Component
}

// Component is public but contains an unexported return type
// this is done with intent as the user should never use ComponentWrapper
// explicitly.
//
//revive:disable-next-line:unexported-return
// Component returns a ComponentWrapper for the given component type T.
// This function is intentionally designed to return an unexported type
// as ComponentWrapper should not be used directly.
func Component[T types.Component]() ComponentWrapper {
var x T
return ComponentWrapper{
Expand Down
13 changes: 5 additions & 8 deletions cardinal/gamestate/redis.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@ import (
"go.opentelemetry.io/otel"
"go.opentelemetry.io/otel/codes"
"go.opentelemetry.io/otel/trace"
ddotel "gopkg.in/DataDog/dd-trace-go.v1/ddtrace/opentelemetry"
ddtracer "gopkg.in/DataDog/dd-trace-go.v1/ddtrace/tracer"

"pkg.world.dev/world-engine/cardinal/codec"
"pkg.world.dev/world-engine/cardinal/types"
Expand Down Expand Up @@ -126,7 +124,7 @@ func (r *RedisStorage) StartTransaction(_ context.Context) (Transaction[string],
}

func (r *RedisStorage) EndTransaction(ctx context.Context) error {
ctx, span := r.tracer.Start(ddotel.ContextWithStartOptions(ctx, ddtracer.Measured()), "redis.transaction.end")
ctx, span := r.tracer.Start(ctx, "redis.transaction.end")
defer span.End()

pipeline, ok := r.currentClient.(redis.Pipeliner)
Expand All @@ -150,7 +148,7 @@ func (r *RedisStorage) EndTransaction(ctx context.Context) error {
// makePipeOfRedisCommands return a pipeliner with all pending state changes to redis ready to be committed in an atomic
// transaction. If an error is returned, no redis changes will have been made.
func (m *EntityCommandBuffer) makePipeOfRedisCommands(ctx context.Context) (PrimitiveStorage[string], error) {
ctx, span := m.tracer.Start(ddotel.ContextWithStartOptions(ctx, ddtracer.Measured()), "ecb.tick.finalize.pipe_make")
ctx, span := m.tracer.Start(ctx, "ecb.tick.finalize.pipe_make")
defer span.End()

pipe, err := m.dbStorage.StartTransaction(ctx)
Expand Down Expand Up @@ -179,15 +177,14 @@ func (m *EntityCommandBuffer) makePipeOfRedisCommands(ctx context.Context) (Prim
}

for _, operation := range operations {
ctx, pipeSpan := m.tracer.Start(ddotel.ContextWithStartOptions(ctx, //nolint:spancheck // false positive
ddtracer.Measured()),
"tick.span.finalize.pipe_make."+operation.name)
ctx, pipeSpan := m.tracer.Start(ctx, "tick.span.finalize.pipe_make."+operation.name)
if err := operation.method(ctx, pipe); err != nil {
span.SetStatus(codes.Error, eris.ToString(err, true))
span.RecordError(err)
pipeSpan.SetStatus(codes.Error, eris.ToString(err, true))
pipeSpan.RecordError(err)
return nil, eris.Wrapf(err, "failed to run step %q", operation.name) //nolint:spancheck // false positive
pipeSpan.End()
return nil, eris.Wrapf(err, "failed to run step %q", operation.name)
}
pipeSpan.End()
}
Expand Down
4 changes: 1 addition & 3 deletions cardinal/gamestate/tick.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,6 @@ import (
"github.com/redis/go-redis/v9"
"github.com/rotisserie/eris"
"go.opentelemetry.io/otel/codes"
ddotel "gopkg.in/DataDog/dd-trace-go.v1/ddtrace/opentelemetry"
ddtracer "gopkg.in/DataDog/dd-trace-go.v1/ddtrace/tracer"
)

// The world tick must be updated in the same atomic transaction as all the state changes
Expand Down Expand Up @@ -37,7 +35,7 @@ func (m *EntityCommandBuffer) GetLastFinalizedTick() (uint64, error) {
// FinalizeTick combines all pending state changes into a single multi/exec redis transactions and commits them
// to the DB.
func (m *EntityCommandBuffer) FinalizeTick(ctx context.Context) error {
ctx, span := m.tracer.Start(ddotel.ContextWithStartOptions(ctx, ddtracer.Measured()), "ecb.tick.finalize")
ctx, span := m.tracer.Start(ctx, "ecb.tick.finalize")
Copy link
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Consider adding span attributes for better observability

While the tracing implementation is correct, consider adding span attributes to provide more context about the operation:

 	ctx, span := m.tracer.Start(ctx, "ecb.tick.finalize")
 	defer span.End()
+	span.SetAttributes(
+		attribute.Int64("pending_arch_ids", int64(len(m.pendingArchIDs))),
+	)

Committable suggestion skipped: line range outside the PR's diff.

defer span.End()

pipe, err := m.makePipeOfRedisCommands(ctx)
Expand Down
70 changes: 26 additions & 44 deletions cardinal/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -27,57 +27,47 @@ require (
github.com/stretchr/testify v1.9.0
github.com/swaggo/swag v1.16.3
github.com/wI2L/jsondiff v0.5.0
go.opentelemetry.io/otel v1.26.0
go.opentelemetry.io/otel/trace v1.26.0
golang.org/x/sync v0.6.0
google.golang.org/grpc v1.63.2
go.opentelemetry.io/otel v1.32.0
go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.32.0
go.opentelemetry.io/otel/sdk v1.32.0
go.opentelemetry.io/otel/trace v1.32.0
golang.org/x/sync v0.9.0
google.golang.org/grpc v1.67.1
google.golang.org/protobuf v1.35.1
gopkg.in/DataDog/dd-trace-go.v1 v1.63.1
gotest.tools/v3 v3.5.1
pkg.world.dev/world-engine/assert v1.0.0
pkg.world.dev/world-engine/rift v1.2.0
pkg.world.dev/world-engine/sign v1.1.0
)

require (
github.com/DataDog/appsec-internal-go v1.5.0 // indirect
github.com/DataDog/datadog-agent/pkg/obfuscate v0.48.0 // indirect
github.com/DataDog/datadog-agent/pkg/remoteconfig/state v0.48.1 // indirect
github.com/DataDog/datadog-go/v5 v5.3.0 // indirect
github.com/DataDog/go-libddwaf/v2 v2.4.2 // indirect
github.com/DataDog/go-tuf v1.0.2-0.5.2 // indirect
github.com/DataDog/gostackparse v0.7.0 // indirect
github.com/DataDog/sketches-go v1.4.2 // indirect
github.com/KyleBanks/depth v1.2.1 // indirect
github.com/Microsoft/go-winio v0.6.1 // indirect
github.com/alicebob/gopher-json v0.0.0-20200520072559-a9ecdc9d1d3a // indirect
github.com/andybalholm/brotli v1.1.0 // indirect
github.com/btcsuite/btcd/btcec/v2 v2.3.2 // indirect
github.com/cespare/xxhash/v2 v2.2.0 // indirect
github.com/cenkalti/backoff/v4 v4.3.0 // indirect
github.com/cespare/xxhash/v2 v2.3.0 // indirect
github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect
github.com/decred/dcrd/dcrec/secp256k1/v4 v4.2.0 // indirect
github.com/dgraph-io/badger/v4 v4.2.0 // indirect
github.com/dgraph-io/badger/v4 v4.2.1-0.20231013074411-fb1b00959581 // indirect
github.com/dgraph-io/ristretto v0.1.1 // indirect
github.com/dgryski/go-farm v0.0.0-20200201041132-a6ae2369ad13 // indirect
github.com/dgryski/go-rendezvous v0.0.0-20200823014737-9f7001d12a5f // indirect
github.com/dustin/go-humanize v1.0.1 // indirect
github.com/ebitengine/purego v0.6.0-alpha.5 // indirect
github.com/fsnotify/fsnotify v1.7.0 // indirect
github.com/go-logr/logr v1.4.1 // indirect
github.com/go-logr/logr v1.4.2 // indirect
github.com/go-logr/stdr v1.2.2 // indirect
github.com/go-openapi/jsonpointer v0.20.0 // indirect
github.com/go-openapi/jsonreference v0.20.0 // indirect
github.com/go-openapi/spec v0.20.8 // indirect
github.com/go-openapi/swag v0.22.4 // indirect
github.com/gogo/protobuf v1.3.2 // indirect
github.com/golang/glog v1.2.0 // indirect
github.com/golang/glog v1.2.2 // indirect
github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect
github.com/golang/protobuf v1.5.4 // indirect
github.com/golang/snappy v0.0.5-0.20220116011046-fa5810519dcb // indirect
github.com/google/flatbuffers v1.12.1 // indirect
github.com/google/flatbuffers v23.5.26+incompatible // indirect
github.com/google/go-cmp v0.6.0 // indirect
github.com/google/pprof v0.0.0-20230817174616-7a8ec2ada47b // indirect
github.com/hashicorp/errwrap v1.1.0 // indirect
github.com/hashicorp/go-multierror v1.1.1 // indirect
github.com/grpc-ecosystem/grpc-gateway/v2 v2.23.0 // indirect
github.com/hashicorp/hcl v1.0.1-vault-5 // indirect
github.com/holiman/uint256 v1.2.4 // indirect
github.com/iancoleman/orderedmap v0.0.0-20190318233801-ac98e3ecb4b0 // indirect
Expand All @@ -90,21 +80,15 @@ require (
github.com/mattn/go-runewidth v0.0.15 // indirect
github.com/mitchellh/mapstructure v1.5.0 // indirect
github.com/naoina/go-stringutil v0.1.0 // indirect
github.com/outcaste-io/ristretto v0.2.3 // indirect
github.com/pelletier/go-toml/v2 v2.1.0 // indirect
github.com/philhofer/fwd v1.1.2 // indirect
github.com/pkg/errors v0.9.1 // indirect
github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect
github.com/puzpuzpuz/xsync/v3 v3.2.0 // indirect
github.com/richardartoul/molecule v1.0.1-0.20221107223329-32cfee06a052 // indirect
github.com/rivo/uniseg v0.4.7 // indirect
github.com/rogpeppe/go-internal v1.11.0 // indirect
github.com/sagikazarmark/locafero v0.4.0 // indirect
github.com/sagikazarmark/slog-shim v0.1.0 // indirect
github.com/savsgio/gotils v0.0.0-20240303185622-093b76447511 // indirect
github.com/secure-systems-lab/go-securesystemslib v0.7.0 // indirect
github.com/sourcegraph/conc v0.3.0 // indirect
github.com/spaolacci/murmur3 v1.1.0 // indirect
github.com/spf13/afero v1.11.0 // indirect
github.com/spf13/cast v1.6.0 // indirect
github.com/subosito/gotenv v1.6.0 // indirect
Expand All @@ -113,25 +97,23 @@ require (
github.com/tidwall/match v1.1.1 // indirect
github.com/tidwall/pretty v1.2.1 // indirect
github.com/tidwall/sjson v1.2.5 // indirect
github.com/tinylib/msgp v1.1.8 // indirect
github.com/valyala/bytebufferpool v1.0.0 // indirect
github.com/valyala/fasthttp v1.52.0 // indirect
github.com/valyala/tcplisten v1.0.0 // indirect
github.com/yuin/gopher-lua v1.1.0 // indirect
go.opencensus.io v0.24.0 // indirect
go.opentelemetry.io/otel/metric v1.26.0 // indirect
go.uber.org/atomic v1.11.0 // indirect
go.uber.org/multierr v1.9.0 // indirect
golang.org/x/crypto v0.22.0 // indirect
golang.org/x/exp v0.0.0-20231110203233-9a3e6036ecaa // indirect
golang.org/x/mod v0.14.0 // indirect
golang.org/x/net v0.23.0 // indirect
golang.org/x/sys v0.21.0 // indirect
golang.org/x/text v0.14.0 // indirect
golang.org/x/time v0.5.0 // indirect
golang.org/x/tools v0.16.1 // indirect
golang.org/x/xerrors v0.0.0-20220907171357-04be3eba64a2 // indirect
google.golang.org/genproto/googleapis/rpc v0.0.0-20240401170217-c3f982113cda // indirect
go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.32.0 // indirect
go.opentelemetry.io/otel/metric v1.32.0 // indirect
go.opentelemetry.io/proto/otlp v1.3.1 // indirect
go.uber.org/multierr v1.11.0 // indirect
golang.org/x/crypto v0.28.0 // indirect
golang.org/x/exp v0.0.0-20240404231335-c0f41cb1a7a0 // indirect
golang.org/x/net v0.30.0 // indirect
golang.org/x/sys v0.27.0 // indirect
golang.org/x/text v0.20.0 // indirect
golang.org/x/tools v0.22.0 // indirect
google.golang.org/genproto/googleapis/api v0.0.0-20241104194629-dd2ea8efbc28 // indirect
google.golang.org/genproto/googleapis/rpc v0.0.0-20241104194629-dd2ea8efbc28 // indirect
gopkg.in/ini.v1 v1.67.0 // indirect
gopkg.in/yaml.v3 v3.0.1 // indirect
)
Loading
Loading