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

[v2] Enable remote sampling extension and include in e2e tests #5802

Merged
merged 1 commit into from
Aug 5, 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
6 changes: 1 addition & 5 deletions .github/workflows/ci-docker-all-in-one.yml
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,8 @@ jobs:
mode:
- name: v1
binary: all-in-one
skip_sampling: false
- name: v2
binary: jaeger
skip_sampling: true

steps:
- name: Harden Runner
Expand Down Expand Up @@ -61,7 +59,7 @@ jobs:
run: |
case ${GITHUB_EVENT_NAME} in
pull_request)
echo "BUILD_FLAGS=-l -D -p linux/amd64" >> ${GITHUB_ENV}
echo "BUILD_FLAGS=-l -D -p linux/$(go env GOARCH)" >> ${GITHUB_ENV}
;;
*)
echo "BUILD_FLAGS=" >> ${GITHUB_ENV}
Expand All @@ -76,5 +74,3 @@ jobs:
env:
DOCKERHUB_TOKEN: ${{ secrets.DOCKERHUB_TOKEN }}
QUAY_TOKEN: ${{ secrets.QUAY_TOKEN }}
# SKIP_SAMPLING is used by integration tests, see https://github.com/jaegertracing/jaeger/issues/5531
SKIP_SAMPLING: ${{ matrix.mode.skip_sampling }}
10 changes: 5 additions & 5 deletions cmd/all-in-one/all_in_one_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -159,11 +159,11 @@ func getAPITrace(t *testing.T) {
}

func getSamplingStrategy(t *testing.T) {
// TODO once jaeger-v2 can pass this test, remove from .github/workflows/ci-all-in-one-build.yml
if os.Getenv("SKIP_SAMPLING") == "true" {
t.Skip("skipping sampling strategy check because SKIP_SAMPLING=true is set")
}
_, body := httpGet(t, agentAddr+getSamplingStrategyURL)
// TODO should we test refreshing the strategy file?

r, body := httpGet(t, agentAddr+getSamplingStrategyURL)
t.Logf("Sampling strategy response: %s", string(body))
require.EqualValues(t, http.StatusOK, r.StatusCode)

var queryResponse api_v2.SamplingStrategyResponse
require.NoError(t, jsonpb.Unmarshal(bytes.NewReader(body), &queryResponse))
Expand Down
32 changes: 18 additions & 14 deletions cmd/jaeger/Dockerfile
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
ARG base_image
ARG debug_image

# ------------- Begin PROD image -------------

FROM $base_image AS release
ARG TARGETARCH
ARG USER_UID=10001
Expand All @@ -14,9 +16,12 @@ EXPOSE 6831/udp
# Agent jaeger.thrift binary
EXPOSE 6832/udp

# Agent config HTTP
# Sampling config HTTP
EXPOSE 5778

# Sampling config gRPC
EXPOSE 5779

# Collector OTLP gRPC
EXPOSE 4317

Expand All @@ -35,16 +40,15 @@ EXPOSE 9411
# Web HTTP
EXPOSE 16686

# Default configuration file for setting sampling strategies
# ENV SAMPLING_STRATEGIES_FILE=/etc/jaeger/sampling_strategies.json
# COPY sampling_strategies.json /etc/jaeger/

COPY jaeger-linux-$TARGETARCH /go/bin/jaeger-linux
COPY jaeger-linux-$TARGETARCH /cmd/jaeger/jaeger-linux
COPY sampling-strategies.json /cmd/jaeger/sampling-strategies.json

VOLUME ["/tmp"]
ENTRYPOINT ["/go/bin/jaeger-linux"]
ENTRYPOINT ["/cmd/jaeger/jaeger-linux"]
USER ${USER_UID}

# ------------- Begin DEBUG image -------------

FROM $debug_image AS debug
ARG TARGETARCH=amd64
ARG USER_UID=10001
Expand All @@ -58,9 +62,12 @@ EXPOSE 6831/udp
# Agent jaeger.thrift binary
EXPOSE 6832/udp

# Agent config HTTP
# Sampling config HTTP
EXPOSE 5778

# Sampling config gRPC
EXPOSE 5779

# Collector OTLP gRPC
EXPOSE 4317

Expand All @@ -82,12 +89,9 @@ EXPOSE 16686
# Delve
EXPOSE 12345

# Default configuration file for setting sampling strategies
# ENV SAMPLING_STRATEGIES_FILE=/etc/jaeger/sampling_strategies.json
# COPY sampling_strategies.json /etc/jaeger/

COPY jaeger-debug-linux-$TARGETARCH /go/bin/jaeger-linux
COPY jaeger-debug-linux-$TARGETARCH /cmd/jaeger/jaeger-linux
COPY sampling-strategies.json /cmd/jaeger/sampling-strategies.json

VOLUME ["/tmp"]
ENTRYPOINT ["/go/bin/dlv", "exec", "/go/bin/jaeger-linux", "--headless", "--listen=:12345", "--api-version=2", "--accept-multiclient", "--log", "--"]
ENTRYPOINT ["/go/bin/dlv", "exec", "/cmd/jaeger/jaeger-linux", "--headless", "--listen=:12345", "--api-version=2", "--accept-multiclient", "--log", "--"]
USER ${USER_UID}
14 changes: 12 additions & 2 deletions cmd/jaeger/internal/all-in-one.yaml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
service:
extensions: [jaeger_storage, jaeger_query]
extensions: [jaeger_storage, jaeger_query, remote_sampling]
pipelines:
traces:
receivers: [otlp, jaeger, zipkin]
Expand All @@ -12,7 +12,7 @@ service:
level: detailed
address: 0.0.0.0:8888
# TODO Initialize telemetery tracer once OTEL released new feature.
# https://github.com/open-telemetry/opentelemetry-collector/issues/10663
# https://github.com/open-telemetry/opentelemetry-collector/issues/10663

extensions:
jaeger_query:
Expand All @@ -24,6 +24,16 @@ extensions:
memory:
max_traces: 100000

remote_sampling:
# We can either use file or adaptive sampling strategy in remote_sampling
file:
path: ./cmd/jaeger/sampling-strategies.json
# adaptive:
# sampling_store: some_store
# initial_sampling_probability: 0.1
http:
grpc:

receivers:
otlp:
protocols:
Expand Down
7 changes: 6 additions & 1 deletion cmd/jaeger/internal/extension/remotesampling/extension.go
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,12 @@ func (ext *rsExtension) startHTTPServer(ctx context.Context, host component.Host
SamplingProvider: ext.strategyProvider,
},
MetricsFactory: metrics.NullFactory,
BasePath: "/api", // TODO is /api correct?

// In v1 the sampling endpoint in the collector was at /api/sampling, because
// the collector reused the same port for multiple services. In v2, the extension
// always uses a separate port, making /api prefix unnecessary. So we mimic the behavior
// of jaeger-agent port 5778 which serves sampling strategies at /sampling endpoint.
BasePath: "",
})
httpMux := http.NewServeMux()
handler.RegisterRoutesWithHTTP(httpMux)
Expand Down
4 changes: 2 additions & 2 deletions cmd/jaeger/internal/extension/remotesampling/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,11 @@ func NewFactory() extension.Factory {
func createDefaultConfig() component.Config {
return &Config{
HTTP: &confighttp.ServerConfig{
Endpoint: ports.PortToHostPort(ports.CollectorHTTP + 100),
Endpoint: ports.PortToHostPort(ports.AgentConfigServerHTTP),
},
GRPC: &configgrpc.ServerConfig{
NetAddr: confignet.AddrConfig{
Endpoint: ports.PortToHostPort(ports.CollectorGRPC + 100),
Endpoint: ports.PortToHostPort(ports.AgentConfigServerHTTP + 1),
Transport: confignet.TransportTypeTCP,
},
},
Expand Down
4 changes: 2 additions & 2 deletions cmd/jaeger/sampling-strategies.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"default_strategy": {
"type": "probabilistic",
"param": 0.1
"param": 1
},
"service_strategies": [
{
Expand All @@ -12,7 +12,7 @@
{
"service": "bar",
"type": "ratelimiting",
"param": 1
"param": 0.9
}
]
}
2 changes: 2 additions & 0 deletions scripts/build-all-in-one-image.sh
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ make build-ui
run_integration_test() {
local image_name="$1"
CID=$(docker run -d -p 16686:16686 -p 5778:5778 "${image_name}:${GITHUB_SHA}")

if ! make all-in-one-integration-test ; then
echo "---- integration test failed unexpectedly ----"
echo "--- check the docker log below for details ---"
Expand Down Expand Up @@ -90,6 +91,7 @@ fi

# build all-in-one image locally for integration test (the -l switch)
bash scripts/build-upload-a-docker-image.sh -l -b -c "${BINARY}" -d "cmd/${BINARY}" -p "${platforms}" -t release

run_integration_test "localhost:5000/$repo"

# build all-in-one image and upload to dockerhub/quay.io
Expand Down
Loading