Skip to content

Commit

Permalink
[v2] Enable remote sampling extension and include in e2e tests (jaege…
Browse files Browse the repository at this point in the history
…rtracing#5802)

## Which problem is this PR solving?
- Resolves jaegertracing#5531

## Description of the changes
- Enable remote sampling extension in v2 all-in-one configuration
- Fix Dockerfile to copy this into the image in the appropriate location
- Change the default ports for the extension to match OTEL & agent
5778/5779
- Re-enable sampling e2e tests for v2 all-in-one

## How was this change tested?
- `make all-in-one-integration-test`
- CI

Signed-off-by: Yuri Shkuro <github@ysh.us>
Signed-off-by: Jared Tan <jian.tan@daocloud.io>
  • Loading branch information
yurishkuro authored and JaredTan95 committed Aug 13, 2024
1 parent c065020 commit 38b8922
Show file tree
Hide file tree
Showing 8 changed files with 48 additions and 31 deletions.
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

0 comments on commit 38b8922

Please sign in to comment.