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

(otelarrowreceiver): add admission control to otlp path #35021

Merged
merged 28 commits into from
Sep 26, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
fb4bbe8
add admission control to otlp path
moh-osman3 Sep 5, 2024
9706583
support deprecated option + changelog + unit tests
moh-osman3 Sep 6, 2024
35d9d09
rename
moh-osman3 Sep 6, 2024
b3feccf
replace statement?
moh-osman3 Sep 6, 2024
36fc7b2
lint
moh-osman3 Sep 6, 2024
75788fd
lint
moh-osman3 Sep 6, 2024
0944757
make crosslink?
moh-osman3 Sep 6, 2024
6eeee4b
lint again
moh-osman3 Sep 6, 2024
692484f
address review feedback
moh-osman3 Sep 6, 2024
2dcdd8e
fix tests
moh-osman3 Sep 6, 2024
4e4a4ed
lint
moh-osman3 Sep 6, 2024
4202afd
add yaml
moh-osman3 Sep 6, 2024
2d26469
fix test after rebase
moh-osman3 Sep 10, 2024
d80fff4
fix test
moh-osman3 Sep 10, 2024
58c525e
change admission config level and name
moh-osman3 Sep 10, 2024
1fae410
gofmt
moh-osman3 Sep 10, 2024
57f9d56
add nil check
moh-osman3 Sep 10, 2024
c04c5bd
rebase + review feedback
moh-osman3 Sep 12, 2024
3469d9a
review feedback
moh-osman3 Sep 17, 2024
c5791ae
indent chlog?
moh-osman3 Sep 17, 2024
3a878ac
update readme link
moh-osman3 Sep 17, 2024
f5f170b
move log warning to newOtelArrowReceiver
moh-osman3 Sep 18, 2024
c748c5e
move mutation to Unmarshal
jmacd Sep 20, 2024
73217bf
Merge branch 'main' of github.com:open-telemetry/opentelemetry-collec…
jmacd Sep 20, 2024
62e8f82
tidy
jmacd Sep 20, 2024
91c1ddc
Merge branch 'main' of github.com:open-telemetry/opentelemetry-collec…
jmacd Sep 23, 2024
3510b3a
Merge branch 'main' of github.com:open-telemetry/opentelemetry-collec…
jmacd Sep 23, 2024
411d5f7
comments
jmacd Sep 24, 2024
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
Prev Previous commit
Next Next commit
support deprecated option + changelog + unit tests
  • Loading branch information
moh-osman3 committed Sep 12, 2024
commit 9706583ca1967f705a36a99326ec1f6465150b1a
27 changes: 27 additions & 0 deletions .chloggen/admission-control-for-otlp.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
# Use this changelog template to create an entry for release notes.

# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
change_type: enhancement
crobert-1 marked this conversation as resolved.
Show resolved Hide resolved

# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver)
component: otelarrowreceiver

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: Add admission control in the otelarrow receiver's standard otlp data path.

# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists.
issues: [35021]

# (Optional) One or more lines of additional information to render under the primary note.
# These lines will be padded with 2 spaces and then inserted directly into the document.
# Use pipe (|) for multiline entries.
subtext:

# If your change doesn't affect end users or the exported elements of any package,
# you should instead start your pull request title with [chore] or use the "Skip Changelog" label.
# Optional: The change log or logs in which this entry should be included.
# e.g. '[user]' or '[user, api]'
# Include 'user' if the change is relevant to end users.
# Include 'api' if there is a change to a library API.
# Default: '[user]'
change_logs: [user]
28 changes: 25 additions & 3 deletions receiver/otelarrowreceiver/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,9 @@ import (

// Protocols is the configuration for the supported protocols.
type Protocols struct {
GRPC configgrpc.ServerConfig `mapstructure:"grpc"`
Arrow ArrowConfig `mapstructure:"arrow"`
Admission AdmissionConfig `mapstructure:"admission"`
GRPC configgrpc.ServerConfig `mapstructure:"grpc"`
Arrow ArrowConfig `mapstructure:"arrow"`
Admission AdmissionConfig `mapstructure:"admission"`
}

type AdmissionConfig struct {
crobert-1 marked this conversation as resolved.
Show resolved Hide resolved
Expand All @@ -38,6 +38,12 @@ type ArrowConfig struct {
// passing through, they will see ResourceExhausted errors.
MemoryLimitMiB uint64 `mapstructure:"memory_limit_mib"`

// Deprecated: This field is no longer supported, use cfg.Admission.AdmissionLimitMiB instead.
AdmissionLimitMiB uint64 `mapstructure:"admission_limit_mib"`

// Deprecated: This field is no longer supported, use cfg.Admission.WaiterLimit instead.
WaiterLimit int64 `mapstructure:"waiter_limit"`

// Zstd settings apply to OTel-Arrow use of gRPC specifically.
Zstd zstd.DecoderConfig `mapstructure:"zstd"`
}
Expand All @@ -57,3 +63,19 @@ func (cfg *ArrowConfig) Validate() error {
}
return nil
}

func (cfg *Config) Validate() error {
crobert-1 marked this conversation as resolved.
Show resolved Hide resolved
if err := cfg.GRPC.Validate(); err != nil {
return err
}
if err := cfg.Arrow.Validate(); err != nil {
return err
}
if cfg.Arrow.AdmissionLimitMiB != 0 {
cfg.Admission.AdmissionLimitMiB = cfg.Arrow.AdmissionLimitMiB
}
if cfg.Arrow.WaiterLimit != 0 {
cfg.Admission.WaiterLimit = cfg.Arrow.WaiterLimit
}
return nil
}
46 changes: 46 additions & 0 deletions receiver/otelarrowreceiver/internal/common/blocking_consumer.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
// Copyright The OpenTelemetry Authors
// SPDX-License-Identifier: Apache-2.0

package common

import(
"context"

"go.opentelemetry.io/collector/consumer"
"go.opentelemetry.io/collector/pdata/plog"
"go.opentelemetry.io/collector/pdata/pmetric"
"go.opentelemetry.io/collector/pdata/ptrace"
)

type BlockingConsumer struct{
block chan struct{}
}

func NewBlockingConsumer() *BlockingConsumer {
return &BlockingConsumer{
block: make(chan struct{}),
}
}

func (bc *BlockingConsumer) ConsumeTraces(ctx context.Context, _ ptrace.Traces) error {
<-bc.block
return nil
}

func (bc *BlockingConsumer) ConsumeMetrics(ctx context.Context, _ pmetric.Metrics) error {
<-bc.block
return nil
}

func (bc *BlockingConsumer) ConsumeLogs(ctx context.Context, _ plog.Logs) error {
<-bc.block
return nil
}

func (bc *BlockingConsumer) Unblock() {
close(bc.block)
}

func (bc *BlockingConsumer) Capabilities() consumer.Capabilities {
return consumer.Capabilities{MutatesData: false}
}
69 changes: 68 additions & 1 deletion receiver/otelarrowreceiver/internal/logs/otlp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@ import (
"context"
"errors"
"net"
"sync"
"testing"
"time"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
Expand All @@ -19,8 +21,16 @@ import (
"go.opentelemetry.io/collector/receiver/receivertest"
"google.golang.org/grpc"
"google.golang.org/grpc/credentials/insecure"
"go.uber.org/multierr"

"github.com/open-telemetry/opentelemetry-collector-contrib/internal/otelarrow/admission"
"github.com/open-telemetry/opentelemetry-collector-contrib/internal/otelarrow/testdata"
"github.com/open-telemetry/opentelemetry-collector-contrib/receiver/otelarrowreceiver/internal/common"
)

const (
maxWaiters = 10
maxBytes = int64(250)
)

func TestExport(t *testing.T) {
Expand Down Expand Up @@ -57,6 +67,61 @@ func TestExport_ErrorConsumer(t *testing.T) {
assert.Equal(t, plogotlp.ExportResponse{}, resp)
}

func TestExport_AdmissionLimitBytesExceeded(t *testing.T) {
ld := testdata.GenerateLogs(10)
logSink := new(consumertest.LogsSink)
req := plogotlp.NewExportRequestFromLogs(ld)

logClient := makeLogsServiceClient(t, logSink)
resp, err := logClient.Export(context.Background(), req)
assert.EqualError(t, err, "rpc error: code = Unknown desc = rejecting request, request size larger than configured limit")
assert.Equal(t, plogotlp.ExportResponse{}, resp)
}

func TestExport_TooManyWaiters(t *testing.T) {
bc := common.NewBlockingConsumer()

logsClient := makeLogsServiceClient(t, bc)
bg := context.Background()
var errs, err error
ld := testdata.GenerateLogs(1)
req := plogotlp.NewExportRequestFromLogs(ld)
var mtx sync.Mutex
numResponses := 0
// Send request that will acquire all of the semaphores bytes and block.
go func() {
_, err = logsClient.Export(bg, req)
mtx.Lock()
errs = multierr.Append(errs, err)
numResponses+=1
mtx.Unlock()
}()

for i := 0; i < maxWaiters+1; i++ {
go func() {
_, err := logsClient.Export(bg, req)
mtx.Lock()
errs = multierr.Append(errs, err)
numResponses+=1
mtx.Unlock()
}()
}

// sleep so all async requests are blocked on semaphore Acquire.
time.Sleep(1 * time.Second)

// unblock and wait for errors to be returned and written.
bc.Unblock()
assert.Eventually(t, func() bool {
mtx.Lock()
defer mtx.Unlock()
errSlice := multierr.Errors(errs)
return numResponses == maxWaiters+2 && len(errSlice) == 1
}, 3 * time.Second, 10 * time.Millisecond)

assert.ErrorContains(t, errs, "too many waiters")
}

func makeLogsServiceClient(t *testing.T, lc consumer.Logs) plogotlp.GRPCClient {
addr := otlpReceiverOnGRPCServer(t, lc)
cc, err := grpc.NewClient(addr.String(), grpc.WithTransportCredentials(insecure.NewCredentials()))
Expand Down Expand Up @@ -84,7 +149,9 @@ func otlpReceiverOnGRPCServer(t *testing.T, lc consumer.Logs) net.Addr {
ReceiverCreateSettings: set,
})
require.NoError(t, err)
r := New(lc, obsrecv)

bq := admission.NewBoundedQueue(maxBytes, maxWaiters)
r := New(lc, obsrecv, bq)
// Now run it as a gRPC server
srv := grpc.NewServer()
plogotlp.RegisterGRPCServer(srv, r)
Expand Down
69 changes: 68 additions & 1 deletion receiver/otelarrowreceiver/internal/metrics/otlp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@ import (
"context"
"errors"
"net"
"sync"
"testing"
"time"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
Expand All @@ -19,8 +21,16 @@ import (
"go.opentelemetry.io/collector/receiver/receivertest"
"google.golang.org/grpc"
"google.golang.org/grpc/credentials/insecure"
"go.uber.org/multierr"

"github.com/open-telemetry/opentelemetry-collector-contrib/internal/otelarrow/admission"
"github.com/open-telemetry/opentelemetry-collector-contrib/internal/otelarrow/testdata"
"github.com/open-telemetry/opentelemetry-collector-contrib/receiver/otelarrowreceiver/internal/common"
)

const (
maxWaiters = 10
maxBytes = int64(250)
)

func TestExport(t *testing.T) {
Expand Down Expand Up @@ -57,6 +67,61 @@ func TestExport_ErrorConsumer(t *testing.T) {
assert.Equal(t, pmetricotlp.ExportResponse{}, resp)
}

func TestExport_AdmissionLimitBytesExceeded(t *testing.T) {
md := testdata.GenerateMetrics(10)
metricSink := new(consumertest.MetricsSink)
req := pmetricotlp.NewExportRequestFromMetrics(md)

metricsClient := makeMetricsServiceClient(t, metricSink)
resp, err := metricsClient.Export(context.Background(), req)
assert.EqualError(t, err, "rpc error: code = Unknown desc = rejecting request, request size larger than configured limit")
assert.Equal(t, pmetricotlp.ExportResponse{}, resp)
}

func TestExport_TooManyWaiters(t *testing.T) {
bc := common.NewBlockingConsumer()

metricsClient := makeMetricsServiceClient(t, bc)
bg := context.Background()
var errs, err error
md := testdata.GenerateMetrics(1)
req := pmetricotlp.NewExportRequestFromMetrics(md)
var mtx sync.Mutex
numResponses := 0
// Send request that will acquire all of the semaphores bytes and block.
go func() {
_, err = metricsClient.Export(bg, req)
mtx.Lock()
errs = multierr.Append(errs, err)
numResponses+=1
mtx.Unlock()
}()

for i := 0; i < maxWaiters+1; i++ {
go func() {
_, err := metricsClient.Export(bg, req)
mtx.Lock()
errs = multierr.Append(errs, err)
numResponses+=1
mtx.Unlock()
}()
}

// sleep so all async requests are blocked on semaphore Acquire.
time.Sleep(1 * time.Second)

// unblock and wait for errors to be returned and written.
bc.Unblock()
assert.Eventually(t, func() bool {
mtx.Lock()
defer mtx.Unlock()
errSlice := multierr.Errors(errs)
return numResponses == maxWaiters+2 && len(errSlice) == 1
}, 3 * time.Second, 10 * time.Millisecond)

assert.ErrorContains(t, errs, "too many waiters")
}

func makeMetricsServiceClient(t *testing.T, mc consumer.Metrics) pmetricotlp.GRPCClient {
addr := otlpReceiverOnGRPCServer(t, mc)

Expand Down Expand Up @@ -85,7 +150,9 @@ func otlpReceiverOnGRPCServer(t *testing.T, mc consumer.Metrics) net.Addr {
ReceiverCreateSettings: set,
})
require.NoError(t, err)
r := New(mc, obsrecv)

bq := admission.NewBoundedQueue(maxBytes, maxWaiters)
r := New(mc, obsrecv, bq)
// Now run it as a gRPC server
srv := grpc.NewServer()
pmetricotlp.RegisterGRPCServer(srv, r)
Expand Down
2 changes: 0 additions & 2 deletions receiver/otelarrowreceiver/internal/trace/otlp.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ package trace // import "github.com/open-telemetry/opentelemetry-collector-contr

import (
"context"
"fmt"

"go.opentelemetry.io/collector/consumer"
"go.opentelemetry.io/collector/pdata/ptrace"
Expand Down Expand Up @@ -47,7 +46,6 @@ func (r *Receiver) Export(ctx context.Context, req ptraceotlp.ExportRequest) (pt
ctx = r.obsrecv.StartTracesOp(ctx)

sizeBytes := int64(r.sizer.TracesSize(req.Traces()))
fmt.Println(sizeBytes)
err := r.boundedQueue.Acquire(ctx, sizeBytes)
if err != nil {
return ptraceotlp.NewExportResponse(), err
Expand Down
Loading