Skip to content

Commit

Permalink
Merge branch 'main' into add-inferred-proxy-spans
Browse files Browse the repository at this point in the history
  • Loading branch information
zarirhamza authored Jan 22, 2025
2 parents 78b3d4b + 241a6f1 commit 39087f1
Show file tree
Hide file tree
Showing 9 changed files with 256 additions and 60 deletions.
26 changes: 0 additions & 26 deletions .github/ISSUE_TEMPLATE/bug-report.md

This file was deleted.

49 changes: 49 additions & 0 deletions .github/ISSUE_TEMPLATE/bug-report.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
name: "Bug Report (Low Priority)"
description: "Create a public Bug Report. Note that these may not be addressed as quickly as the helpdesk and that looking up account information will be difficult."
title: "[BUG]: "
labels: bug
body:
- type: input
attributes:
label: Tracer Version(s)
description: "Version(s) of the tracer affected by this bug. If you aren't using the [latest version](https://github.com/DataDog/dd-trace-go/releases) of dd-trace-go, try upgrading first to see if your issue has already been resolved."
placeholder: 1.70.0
validations:
required: true

- type: input
attributes:
label: Go Version(s)
description: "Version(s) of Go (`go version`) that you've encountered this bug with."
placeholder: "go version go1.23.2 darwin/arm64"
validations:
required: true

- type: textarea
attributes:
label: Bug Report
description: Please add a clear and concise description of the bug here
validations:
required: true

- type: textarea
attributes:
label: Reproduction Code
description: Please add code here to help us reproduce the problem
validations:
required: false

- type: textarea
attributes:
label: Error Logs
description: "Please provide any error logs from the tracer (`DD_TRACE_DEBUG=true` can help)"
validations:
required: false

- type: input
attributes:
label: Go Env Output
description: "Provide the output from `go env`"
placeholder: "GOARCH='arm64' ... GOVERSION='go1.23.2' ..."
validations:
required: false
8 changes: 8 additions & 0 deletions .github/ISSUE_TEMPLATE/config.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
blank_issues_enabled: false
contact_links:
- name: Bug Report (High Priority)
url: https://help.datadoghq.com/hc/en-us/requests/new?tf_1260824651490=pt_product_type:apm&tf_1900004146284=pt_apm_language:go
about: Create an expedited Bug Report via the helpdesk (no login required). This will allow us to look up your account and allows you to provide additional information in private.
- name: Feature Request (High Priority)
url: https://help.datadoghq.com/hc/en-us/requests/new?tf_1260824651490=pt_product_type:apm&tf_1900004146284=pt_apm_language:go&tf_1260825272270=pt_apm_category_feature_request
about: Create an expedited Feature Request via the helpdesk (no login required). This helps with prioritization and allows you to provide additional information in private.
12 changes: 0 additions & 12 deletions .github/ISSUE_TEMPLATE/feature-request.md

This file was deleted.

50 changes: 50 additions & 0 deletions .github/ISSUE_TEMPLATE/feature-request.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
name: Feature Request (Low Priority)
description: Create a public Feature Request. Note that these may not be addressed as quickly as the helpdesk and that looking up account information will be difficult.
title: "[FEATURE]: "
labels: enhancement
body:
- type: input
attributes:
label: Package Name
description: "If your feature request is to add instrumentation support for a package please provide the name here"
placeholder: doctrine/orm
validations:
required: false

- type: input
attributes:
label: Package Version(s)
description: "If your feature request is to add instrumentation support for a package please provide the version you use"
placeholder: 3.3.0
validations:
required: false

- type: textarea
attributes:
label: Describe the feature you'd like
description: A clear and concise description of what you want to happen.
validations:
required: true

- type: textarea
attributes:
label: Is your feature request related to a problem?
description: |
Please add a clear and concise description of your problem.
E.g. I'm unable to instrument my database queries...
validations:
required: false

- type: textarea
attributes:
label: Describe alternatives you've considered
description: A clear and concise description of any alternative solutions or features you've considered
validations:
required: false

- type: textarea
attributes:
label: Additional context
description: Add any other context or screenshots about the feature request here
validations:
required: false
4 changes: 2 additions & 2 deletions .github/workflows/orchestrion.yml
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ jobs:
uses: actions/checkout@v4
with:
repository: ${{ inputs.orchestrion-version != '' && 'DataDog/dd-trace-go' || github.repository }}
ref: ${{ inputs.orchestrion-version != '' && 'romain.marcadier/APPSEC-55160/orchestrion' || github.sha }} # TODO: Change to `main` in workflow_call case
ref: ${{ inputs.orchestrion-version != '' && 'main' || github.sha }}
- name: Setup Go
uses: actions/setup-go@v5
with:
Expand Down Expand Up @@ -97,7 +97,7 @@ jobs:
with:
path: ${{ github.workspace }}/dd-trace-go
repository: ${{ inputs.orchestrion-version != '' && 'DataDog/dd-trace-go' || github.repository }}
ref: ${{ inputs.orchestrion-version != '' && 'romain.marcadier/APPSEC-55160/orchestrion' || github.sha }} # TODO: Change to `main` in workflow_call case
ref: ${{ inputs.orchestrion-version != '' && 'main' || github.sha }}
# If we're in workflow_dispatch/call, maybe we need to up/downgrade orchestrion
- name: Check out orchestrion
if: inputs.orchestrion-version != ''
Expand Down
89 changes: 85 additions & 4 deletions .gitlab/macrobenchmarks.yml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
variables:
BENCHMARKS_CI_IMAGE: 486234852809.dkr.ecr.us-east-1.amazonaws.com/ci/benchmarking-platform:go-go-prof-app
BENCHMARKS_CI_IMAGE: 486234852809.dkr.ecr.us-east-1.amazonaws.com/ci/benchmarking-platform:go-go-prof-app-and-serviceextensions-0001

.benchmarks:
.benchmarks-default:
stage: macrobenchmarks
needs: []
tags: ["runner:apm-k8s-same-cpu"]
Expand Down Expand Up @@ -57,12 +57,12 @@ variables:
#

.go123-benchmarks:
extends: .benchmarks
extends: .benchmarks-default
variables:
GO_VERSION: "1.23.0"

.go122-benchmarks:
extends: .benchmarks
extends: .benchmarks-default
variables:
GO_VERSION: "1.22.5"

Expand Down Expand Up @@ -176,3 +176,84 @@ go123-profile-trace-asm:
ENABLE_PROFILING: "true"
ENABLE_APPSEC: "true"
DD_PROFILING_EXECUTION_TRACE_ENABLED: "false"

#
# Macro benchmarks for Service Extensions
# (using Envoy External Processing)
#

.benchmarks-serviceextensions:
stage: macrobenchmarks
needs: []
tags: ["runner:apm-k8s-same-cpu"]
timeout: 1h
rules:
- if: $CI_COMMIT_REF_NAME == "main"
when: always
- when: manual
# If you have a problem with Gitlab cache, see Troubleshooting section in Benchmarking Platform docs
image: $BENCHMARKS_CI_IMAGE
script:
- git clone --branch go/go-prof-app https://gitlab-ci-token:${CI_JOB_TOKEN}@gitlab.ddbuild.io/DataDog/benchmarking-platform platform && cd platform
- bp-runner bp-runner.envoy_serviceextension.yml --debug
artifacts:
name: "artifacts"
when: always
paths:
- platform/artifacts-se/
expire_in: 3 months
variables:
FF_USE_LEGACY_KUBERNETES_EXECUTION_STRATEGY: "true" # Important tweak for stability of benchmarks
GO_VERSION: "1.23.0"
ARTIFACTS_DIR: "./artifacts-se"

# Workaround: Currently we're not running the benchmarks on every PR, but GitHub still shows them as pending.
# By marking the benchmarks as allow_failure, this should go away. (This workaround should be removed once the
# benchmarks get changed to run on every PR)
allow_failure: true

retry:
max: 2
when:
- unknown_failure
- data_integrity_failure
- runner_system_failure
- scheduler_failure
- api_failure

# Scenario with external processor, webserver without tracer
se-ext_proc-appsec:
extends: .benchmarks-serviceextensions
variables:
EXT_PROC: true
ENABLE_APPSEC: true
TRACER: false

se-ext_proc-only-tracing:
extends: .benchmarks-serviceextensions
variables:
EXT_PROC: true
ENABLE_APPSEC: false
TRACER: false

# Scenarios without external processor, webserver with tracer
se-tracer-no-ext_proc-appsec:
extends: .benchmarks-serviceextensions
variables:
EXT_PROC: false
ENABLE_APPSEC: true
TRACER: true

se-tracer-no-ext_proc-only-tracing:
extends: .benchmarks-serviceextensions
variables:
EXT_PROC: false
ENABLE_APPSEC: false
TRACER: true

# Scenario without tracer, only direct connection through envoy to the webserver
se-no-tracer-no-ext_proc:
extends: .benchmarks-serviceextensions
variables:
EXT_PROC: false
TRACER: false
2 changes: 1 addition & 1 deletion contrib/log/slog/slog.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ func (h *handler) Handle(ctx context.Context, rec slog.Record) error {
// In case the user has created group loggers, we ignore those and
// set them at the root level.
span, ok := tracer.SpanFromContext(ctx)
if ok {
if ok && span.Context().TraceID() != 0 {
traceID := strconv.FormatUint(span.Context().TraceID(), 10)
spanID := strconv.FormatUint(span.Context().SpanID(), 10)

Expand Down
76 changes: 61 additions & 15 deletions contrib/log/slog/slog_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,29 @@ func assertLogEntry(t *testing.T, rawEntry, wantMsg, wantLevel string, span trac
}
}

func assertLogEntryNoTrace(t *testing.T, rawEntry, wantMsg, wantLevel string) {
t.Helper()

t.Log(rawEntry)

var entry map[string]interface{}
err := json.Unmarshal([]byte(rawEntry), &entry)
require.NoError(t, err)
require.NotEmpty(t, entry)

assert.Equal(t, wantMsg, entry["msg"])
assert.Equal(t, wantLevel, entry["level"])
assert.NotEmpty(t, entry["time"])

assert.NotContains(t, entry, ext.LogKeyTraceID)
assert.NotContains(t, entry, ext.LogKeySpanID)
}

func testLogger(t *testing.T, createLogger func(b io.Writer) *slog.Logger, assertExtra func(t *testing.T, entry map[string]interface{})) {
tracer.Start(tracer.WithLogger(internallog.DiscardLogger{}))
tracer.Start(
tracer.WithTraceEnabled(true),
tracer.WithLogger(internallog.DiscardLogger{}),
)
defer tracer.Stop()

// create the application logger
Expand All @@ -74,25 +95,50 @@ func testLogger(t *testing.T, createLogger func(b io.Writer) *slog.Logger, asser
assertLogEntry(t, logs[1], "this is an error log with tracing information", "ERROR", span, assertExtra)
}

func TestNewJSONHandler(t *testing.T) {
testLogger(
t,
func(w io.Writer) *slog.Logger {
return slog.New(NewJSONHandler(w, nil))
},
nil,
func testLoggerNoTrace(t *testing.T, createLogger func(b io.Writer) *slog.Logger) {
tracer.Start(
tracer.WithTraceEnabled(false),
tracer.WithLogger(internallog.DiscardLogger{}),
)
defer tracer.Stop()

// create the application logger
var b bytes.Buffer
logger := createLogger(&b)

// start a new span
span, ctx := tracer.StartSpanFromContext(context.Background(), "test")
defer span.Finish()

// log a message using the context containing span information
logger.Log(ctx, slog.LevelInfo, "this is an info log with tracing information")
logger.Log(ctx, slog.LevelError, "this is an error log with tracing information")

logs := strings.Split(
strings.TrimRight(b.String(), "\n"),
"\n",
)
// assert log entries contain trace information
require.Len(t, logs, 2)
assertLogEntryNoTrace(t, logs[0], "this is an info log with tracing information", "INFO")
assertLogEntryNoTrace(t, logs[1], "this is an error log with tracing information", "ERROR")
}

func TestNewJSONHandler(t *testing.T) {
createLogger := func(w io.Writer) *slog.Logger {
return slog.New(NewJSONHandler(w, nil))
}
testLogger(t, createLogger, nil)
testLoggerNoTrace(t, createLogger)
}

func TestWrapHandler(t *testing.T) {
t.Run("testLogger", func(t *testing.T) {
testLogger(
t,
func(w io.Writer) *slog.Logger {
return slog.New(WrapHandler(slog.NewJSONHandler(w, nil)))
},
nil,
)
createLogger := func(w io.Writer) *slog.Logger {
return slog.New(WrapHandler(slog.NewJSONHandler(w, nil)))
}
testLogger(t, createLogger, nil)
testLoggerNoTrace(t, createLogger)
})

t.Run("slogtest", func(t *testing.T) {
Expand Down

0 comments on commit 39087f1

Please sign in to comment.