From 6f5d7de0eacf88f9885c6f8158e2f4ffb5a28bc0 Mon Sep 17 00:00:00 2001 From: Mend Renovate Date: Sun, 4 Aug 2024 19:02:14 +0200 Subject: [PATCH 01/12] Update module golang.org/x/sys to v0.23.0 (#5800) --- go.mod | 2 +- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index d6441064d2a..a4ecf4d1a5f 100644 --- a/go.mod +++ b/go.mod @@ -88,7 +88,7 @@ require ( go.uber.org/goleak v1.3.0 go.uber.org/zap v1.27.0 golang.org/x/net v0.27.0 - golang.org/x/sys v0.22.0 + golang.org/x/sys v0.23.0 google.golang.org/grpc v1.65.0 google.golang.org/protobuf v1.34.2 gopkg.in/yaml.v3 v3.0.1 diff --git a/go.sum b/go.sum index 1ec758aa397..bb5210e9f6e 100644 --- a/go.sum +++ b/go.sum @@ -782,8 +782,8 @@ golang.org/x/sys v0.0.0-20221010170243-090e33056c14/go.mod h1:oPkhp1MJrh7nUepCBc golang.org/x/sys v0.5.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.8.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.11.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.22.0 h1:RI27ohtqKCnwULzJLqkv897zojh5/DwS/ENaMzUOaWI= -golang.org/x/sys v0.22.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= +golang.org/x/sys v0.23.0 h1:YfKFowiIMvtgl1UERQoTPPToxltDeZfbj4H7dVUCwmM= +golang.org/x/sys v0.23.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8= golang.org/x/term v0.5.0/go.mod h1:jMB1sMXY+tzblOD4FWmEbocvup2/aLOaQEp7JmGp78k= From b76dc65f0a3fcbedb912f2ce30616daf8a73bbb2 Mon Sep 17 00:00:00 2001 From: Yuri Shkuro Date: Mon, 5 Aug 2024 16:07:36 -0300 Subject: [PATCH 02/12] [v2] Enable remote sampling extension and include in e2e tests (#5802) ## Which problem is this PR solving? - Resolves #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/workflows/ci-docker-all-in-one.yml | 6 +--- cmd/all-in-one/all_in_one_test.go | 10 +++--- cmd/jaeger/Dockerfile | 32 +++++++++++-------- cmd/jaeger/internal/all-in-one.yaml | 14 ++++++-- .../extension/remotesampling/extension.go | 7 +++- .../extension/remotesampling/factory.go | 4 +-- cmd/jaeger/sampling-strategies.json | 4 +-- scripts/build-all-in-one-image.sh | 2 ++ 8 files changed, 48 insertions(+), 31 deletions(-) diff --git a/.github/workflows/ci-docker-all-in-one.yml b/.github/workflows/ci-docker-all-in-one.yml index 7d5548bd130..6874f737af1 100644 --- a/.github/workflows/ci-docker-all-in-one.yml +++ b/.github/workflows/ci-docker-all-in-one.yml @@ -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 @@ -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} @@ -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 }} diff --git a/cmd/all-in-one/all_in_one_test.go b/cmd/all-in-one/all_in_one_test.go index afb80ec6f53..9d3cc7a1057 100644 --- a/cmd/all-in-one/all_in_one_test.go +++ b/cmd/all-in-one/all_in_one_test.go @@ -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)) diff --git a/cmd/jaeger/Dockerfile b/cmd/jaeger/Dockerfile index 525de6bacce..c9dcb240368 100644 --- a/cmd/jaeger/Dockerfile +++ b/cmd/jaeger/Dockerfile @@ -1,6 +1,8 @@ ARG base_image ARG debug_image +# ------------- Begin PROD image ------------- + FROM $base_image AS release ARG TARGETARCH ARG USER_UID=10001 @@ -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 @@ -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 @@ -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 @@ -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} diff --git a/cmd/jaeger/internal/all-in-one.yaml b/cmd/jaeger/internal/all-in-one.yaml index 79e939be227..43540cf66f0 100644 --- a/cmd/jaeger/internal/all-in-one.yaml +++ b/cmd/jaeger/internal/all-in-one.yaml @@ -1,5 +1,5 @@ service: - extensions: [jaeger_storage, jaeger_query] + extensions: [jaeger_storage, jaeger_query, remote_sampling] pipelines: traces: receivers: [otlp, jaeger, zipkin] @@ -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: @@ -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: diff --git a/cmd/jaeger/internal/extension/remotesampling/extension.go b/cmd/jaeger/internal/extension/remotesampling/extension.go index 58552d0eaab..e282c17eb9c 100644 --- a/cmd/jaeger/internal/extension/remotesampling/extension.go +++ b/cmd/jaeger/internal/extension/remotesampling/extension.go @@ -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) diff --git a/cmd/jaeger/internal/extension/remotesampling/factory.go b/cmd/jaeger/internal/extension/remotesampling/factory.go index e3a1c9a5756..b0575bddbaf 100644 --- a/cmd/jaeger/internal/extension/remotesampling/factory.go +++ b/cmd/jaeger/internal/extension/remotesampling/factory.go @@ -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, }, }, diff --git a/cmd/jaeger/sampling-strategies.json b/cmd/jaeger/sampling-strategies.json index 6928e6d0436..c4645932d37 100644 --- a/cmd/jaeger/sampling-strategies.json +++ b/cmd/jaeger/sampling-strategies.json @@ -1,7 +1,7 @@ { "default_strategy": { "type": "probabilistic", - "param": 0.1 + "param": 1 }, "service_strategies": [ { @@ -12,7 +12,7 @@ { "service": "bar", "type": "ratelimiting", - "param": 1 + "param": 0.9 } ] } diff --git a/scripts/build-all-in-one-image.sh b/scripts/build-all-in-one-image.sh index 1f80e77087b..f693b0e1398 100755 --- a/scripts/build-all-in-one-image.sh +++ b/scripts/build-all-in-one-image.sh @@ -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 ---" @@ -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 From 0524ae6df8acac17e8ed52af9b86e6dee85a5e5e Mon Sep 17 00:00:00 2001 From: Jonah Kowall Date: Tue, 6 Aug 2024 11:39:43 -0400 Subject: [PATCH 03/12] Prepare release v1.60.0 (#5804) Prepares Jaeger for the 1.60.0 release. Updates changelog, jaeger-ui and rotates release managers. --------- Signed-off-by: Jonah Kowall Co-authored-by: Yuri Shkuro --- CHANGELOG.md | 46 ++++++++++++++++++++++++++++++++++++++++++++++ RELEASE.md | 2 +- jaeger-ui | 2 +- 3 files changed, 48 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 73127e0511c..6e30a7c2c08 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,52 @@ run `make changelog` to generate content +1.60.0 (2024-08-06) +------------------- + +### Backend Changes + +#### ⛔ Breaking Changes + +* Completely remove "grpc-plugin" as storage type ([@yurishkuro](https://github.com/yurishkuro) in [#5741](https://github.com/jaegertracing/jaeger/pull/5741)) + +#### 🐞 Bug fixes, Minor Improvements + +* Do not use image tag without version ([@yurishkuro](https://github.com/yurishkuro) in [#5783](https://github.com/jaegertracing/jaeger/pull/5783)) +* Only attach :latest tag to versioned images from main ([@yurishkuro](https://github.com/yurishkuro) in [#5781](https://github.com/jaegertracing/jaeger/pull/5781)) +* Add references to jaeger v2 ([@yurishkuro](https://github.com/yurishkuro) in [#5779](https://github.com/jaegertracing/jaeger/pull/5779)) +* Ensure hotrod image is published at the end of e2e test ([@yurishkuro](https://github.com/yurishkuro) in [#5764](https://github.com/jaegertracing/jaeger/pull/5764)) +* [bug] [hotrod] delay env var mapping until logger is initialized ([@yurishkuro](https://github.com/yurishkuro) in [#5760](https://github.com/jaegertracing/jaeger/pull/5760)) +* Make otlp receiver listen on all ips again ([@yurishkuro](https://github.com/yurishkuro) in [#5739](https://github.com/jaegertracing/jaeger/pull/5739)) +* [hotrod] fix connectivity in docker compose ([@yurishkuro](https://github.com/yurishkuro) in [#5734](https://github.com/jaegertracing/jaeger/pull/5734)) + +#### 🚧 Experimental Features + +* [v2] enable remote sampling extension and include in e2e tests ([@yurishkuro](https://github.com/yurishkuro) in [#5802](https://github.com/jaegertracing/jaeger/pull/5802)) +* Ensure similar naming for storage write metrics ([@Wise-Wizard](https://github.com/Wise-Wizard) in [#5798](https://github.com/jaegertracing/jaeger/pull/5798)) +* [v2] ensure similar naming for query service metrics ([@Wise-Wizard](https://github.com/Wise-Wizard) in [#5785](https://github.com/jaegertracing/jaeger/pull/5785)) +* Configure otel collector to observe internal telemetry ([@Wise-Wizard](https://github.com/Wise-Wizard) in [#5752](https://github.com/jaegertracing/jaeger/pull/5752)) +* Add kafka exporter and receiver configuration ([@joeyyy09](https://github.com/joeyyy09) in [#5703](https://github.com/jaegertracing/jaeger/pull/5703)) +* Enable spm in jaeger v2 ([@FlamingSaint](https://github.com/FlamingSaint) in [#5681](https://github.com/jaegertracing/jaeger/pull/5681)) +* [jaeger-v2] add `remotesampling` extension ([@Pushkarm029](https://github.com/Pushkarm029) in [#5389](https://github.com/jaegertracing/jaeger/pull/5389)) +* Created telset for remote-storage component ([@Wise-Wizard](https://github.com/Wise-Wizard) in [#5731](https://github.com/jaegertracing/jaeger/pull/5731)) + +#### 👷 CI Improvements + +* Unpin codeql actions ([@yurishkuro](https://github.com/yurishkuro) in [#5787](https://github.com/jaegertracing/jaeger/pull/5787)) +* Skip building hotrod for all platforms for pull requests ([@Manoramsharma](https://github.com/Manoramsharma) in [#5765](https://github.com/jaegertracing/jaeger/pull/5765)) +* Add a threshold for expected zero values in the spm script ([@FlamingSaint](https://github.com/FlamingSaint) in [#5753](https://github.com/jaegertracing/jaeger/pull/5753)) +* [v2] add e2e test with memory store ([@yurishkuro](https://github.com/yurishkuro) in [#5751](https://github.com/jaegertracing/jaeger/pull/5751)) +* Rationalize naming of gha workflow files ([@yurishkuro](https://github.com/yurishkuro) in [#5750](https://github.com/jaegertracing/jaeger/pull/5750)) + + +### 📊 UI Changes + +#### 🐞 Bug fixes, Minor Improvements + +* Allow uploading json-per-line otlp data ([@BenzeneAlcohol](https://github.com/BenzeneAlcohol) in [#2380](https://github.com/jaegertracing/jaeger-ui/pull/2380)) + + 1.59.0 (2024-07-10) ------------------- diff --git a/RELEASE.md b/RELEASE.md index 559a836b7f7..ebf2f3353db 100644 --- a/RELEASE.md +++ b/RELEASE.md @@ -60,8 +60,8 @@ Here are the release managers for future versions with the tentative release dat | Version | Release Manager | Tentative release date | |---------|-----------------|------------------------| -| 1.60.0 | @jkowall | 7 August 2024 | | 1.61.0 | @yurishkuro | 3 September 2024 | | 1.62.0 | @albertteoh | 2 October 2024 | | 1.63.0 | @pavolloffay | 5 November 2024 | | 1.64.0 | @joe-elliott | 4 December 2024 | +| 1.65.0 | @jkowall | 8 January 2025 | diff --git a/jaeger-ui b/jaeger-ui index 1704a9a66ae..3b093f81dec 160000 --- a/jaeger-ui +++ b/jaeger-ui @@ -1 +1 @@ -Subproject commit 1704a9a66ae780ec6bb2f524116176aa9c8a23f8 +Subproject commit 3b093f81dec59c6bb04daad96b1b77bd03a29e4a From 3bc0d35849ac78fe888b33f4a353bc6ee78efa78 Mon Sep 17 00:00:00 2001 From: Yuri Shkuro Date: Tue, 6 Aug 2024 23:59:16 -0300 Subject: [PATCH 04/12] Update expected codecov flags count to 19 (#5811) ## Which problem is this PR solving? We are now running 19 different uploads of codecov image However, the current configuration was only waiting for 11, which resulted in false positives like this: image I.e. a comment about non-covered code which is actually covered, but its result uploads came after the 11 other results. ## Description of the changes - Bump expected count to 19, the current number of distinct flags/uploads. Signed-off-by: Yuri Shkuro --- .codecov.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.codecov.yml b/.codecov.yml index 5bb02757813..19be8c6927e 100644 --- a/.codecov.yml +++ b/.codecov.yml @@ -1,7 +1,7 @@ codecov: notify: require_ci_to_pass: yes - after_n_builds: 11 + after_n_builds: 19 strict_yaml_branch: main # only use the latest copy on the main branch ignore: From 052c89c24e257ddc5d74f17757430d2f2a859d76 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 7 Aug 2024 09:58:15 -0300 Subject: [PATCH 05/12] Bump golang.org/x/net from 0.27.0 to 0.28.0 (#5814) --- go.mod | 6 +++--- go.sum | 20 ++++++++++---------- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/go.mod b/go.mod index a4ecf4d1a5f..d258287fdbf 100644 --- a/go.mod +++ b/go.mod @@ -87,7 +87,7 @@ require ( go.uber.org/automaxprocs v1.5.3 go.uber.org/goleak v1.3.0 go.uber.org/zap v1.27.0 - golang.org/x/net v0.27.0 + golang.org/x/net v0.28.0 golang.org/x/sys v0.23.0 google.golang.org/grpc v1.65.0 google.golang.org/protobuf v1.34.2 @@ -226,9 +226,9 @@ require ( go.opentelemetry.io/proto/otlp v1.3.1 // indirect go.uber.org/atomic v1.11.0 // indirect go.uber.org/multierr v1.11.0 // indirect - golang.org/x/crypto v0.25.0 // indirect + golang.org/x/crypto v0.26.0 // indirect golang.org/x/exp v0.0.0-20240506185415-9bf2ced13842 // indirect - golang.org/x/text v0.16.0 // indirect + golang.org/x/text v0.17.0 // indirect gonum.org/v1/gonum v0.15.0 // indirect google.golang.org/genproto/googleapis/api v0.0.0-20240701130421-f6361c86f094 // indirect google.golang.org/genproto/googleapis/rpc v0.0.0-20240701130421-f6361c86f094 // indirect diff --git a/go.sum b/go.sum index bb5210e9f6e..820ee6e69e7 100644 --- a/go.sum +++ b/go.sum @@ -703,8 +703,8 @@ golang.org/x/crypto v0.0.0-20201112155050-0c6587e931a9/go.mod h1:LzIPMQfyMNhhGPh golang.org/x/crypto v0.0.0-20210921155107-089bfa567519/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc= golang.org/x/crypto v0.0.0-20220214200702-86341886e292/go.mod h1:IxCIyHEi3zRg3s0A5j5BB6A9Jmi73HwBIUl50j+osU4= golang.org/x/crypto v0.6.0/go.mod h1:OFC/31mSvZgRz0V1QTNCzfAI1aIRzbiufJtkMIlEp58= -golang.org/x/crypto v0.25.0 h1:ypSNr+bnYL2YhwoMt2zPxHFmbAN1KZs/njMG3hxUp30= -golang.org/x/crypto v0.25.0/go.mod h1:T+wALwcMOSE0kXgUAnPAHqTLW+XHgcELELW8VaDgm/M= +golang.org/x/crypto v0.26.0 h1:RrRspgV4mU+YwB4FYnuBoKsUapNIL5cohGAmSH3azsw= +golang.org/x/crypto v0.26.0/go.mod h1:GY7jblb9wI+FOo5y8/S2oY4zWP07AkOJ4+jxCqdqn54= golang.org/x/exp v0.0.0-20180321215751-8460e604b9de/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= golang.org/x/exp v0.0.0-20180807140117-3d87b88a115f/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= golang.org/x/exp v0.0.0-20190121172915-509febef88a4/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= @@ -745,8 +745,8 @@ golang.org/x/net v0.0.0-20220225172249-27dd8689420f/go.mod h1:CfG3xpIq0wQ8r1q4Su golang.org/x/net v0.0.0-20220722155237-a158d28d115b/go.mod h1:XRhObCWvk6IyKnWLug+ECip1KBveYUHfp+8e9klMJ9c= golang.org/x/net v0.6.0/go.mod h1:2Tu9+aMcznHK/AK1HMvgo6xiTLG5rD5rZLDS+rp2Bjs= golang.org/x/net v0.7.0/go.mod h1:2Tu9+aMcznHK/AK1HMvgo6xiTLG5rD5rZLDS+rp2Bjs= -golang.org/x/net v0.27.0 h1:5K3Njcw06/l2y9vpGCSdcxWOYHOUk3dVNGDXN+FvAys= -golang.org/x/net v0.27.0/go.mod h1:dDi0PyhWNoiUOrAS8uXv/vnScO4wnHQO4mj9fn/RytE= +golang.org/x/net v0.28.0 h1:a9JDOJc5GMUJ0+UDqmLT86WiEy7iWyIhz8gz8E4e5hE= +golang.org/x/net v0.28.0/go.mod h1:yqtgsTWOOnlGLG9GFRrK3++bGOUEkNBoHZc8MEDWPNg= golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U= golang.org/x/oauth2 v0.21.0 h1:tsimM75w1tF/uws5rbeHzIWxEqElMehnc+iW793zsZs= golang.org/x/oauth2 v0.21.0/go.mod h1:XYTD2NtWslqkgxebSiOHnXEap4TF09sJSc7H1sXbhtI= @@ -756,8 +756,8 @@ golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJ golang.org/x/sync v0.0.0-20190911185100-cd5d95a43a6e/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20201020160332-67f06af15bc9/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20220722155255-886fb9371eb4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= -golang.org/x/sync v0.7.0 h1:YsImfSBoP9QPYL0xyKJPq0gcaJdG3rInoqxTWbfQu9M= -golang.org/x/sync v0.7.0/go.mod h1:Czt+wKu1gCyEFDUtn0jG5QVvpJ6rzVqr5aXyt9drQfk= +golang.org/x/sync v0.8.0 h1:3NFvSEYkUoMifnESzZl15y791HH1qU2xm6eCJU5ZPXQ= +golang.org/x/sync v0.8.0/go.mod h1:Czt+wKu1gCyEFDUtn0jG5QVvpJ6rzVqr5aXyt9drQfk= golang.org/x/sys v0.0.0-20180830151530-49385e6e1522/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20180909124046-d0be0721c37e/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= @@ -787,16 +787,16 @@ golang.org/x/sys v0.23.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8= golang.org/x/term v0.5.0/go.mod h1:jMB1sMXY+tzblOD4FWmEbocvup2/aLOaQEp7JmGp78k= -golang.org/x/term v0.22.0 h1:BbsgPEJULsl2fV/AT3v15Mjva5yXKQDyKf+TbDz7QJk= -golang.org/x/term v0.22.0/go.mod h1:F3qCibpT5AMpCRfhfT53vVJwhLtIVHhB9XDjfFvnMI4= +golang.org/x/term v0.23.0 h1:F6D4vR+EHoL9/sWAWgAR1H2DcHr4PareCbAaCo1RpuU= +golang.org/x/term v0.23.0/go.mod h1:DgV24QBUrK6jhZXl+20l6UWznPlwAHm1Q1mGHtydmSk= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= golang.org/x/text v0.3.6/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= golang.org/x/text v0.3.7/go.mod h1:u+2+/6zg+i71rQMx5EYifcz6MCKuco9NR6JIITiCfzQ= golang.org/x/text v0.3.8/go.mod h1:E6s5w1FMmriuDzIBO73fBruAKo1PCIq6d2Q6DHfQ8WQ= golang.org/x/text v0.7.0/go.mod h1:mrYo+phRRbMaCq/xk9113O4dZlRixOauAjOtrjsXDZ8= -golang.org/x/text v0.16.0 h1:a94ExnEXNtEwYLGJSIUxnWoxoRz/ZcCsV63ROupILh4= -golang.org/x/text v0.16.0/go.mod h1:GhwF1Be+LQoKShO3cGOHzqOgRrGaYc9AvblQOmPVHnI= +golang.org/x/text v0.17.0 h1:XtiM5bkSOt+ewxlOE/aE/AKEHibwj/6gvWMl9Rsh0Qc= +golang.org/x/text v0.17.0/go.mod h1:BuEKDfySbSR4drPmRPG/7iBdf8hvFMuRexcpahXilzY= golang.org/x/time v0.5.0 h1:o7cqy6amK/52YcAKIPlM3a+Fpj35zvRj2TP+e1xFSfk= golang.org/x/time v0.5.0/go.mod h1:3BpzKBy/shNhVucY/MWOyx10tF3SFh9QdLuxbVysPQM= golang.org/x/tools v0.0.0-20180525024113-a5b4c53f6e8b/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= From eaacbf0a40da0c3d55bfeed8d6894f14dca1d4fc Mon Sep 17 00:00:00 2001 From: Saransh Shankar <103821431+Wise-Wizard@users.noreply.github.com> Date: Thu, 8 Aug 2024 06:35:49 +0530 Subject: [PATCH 06/12] Added _total suffix to OTEL counter metrics. (#5810) **Which problem is this PR solving?** This PR addresses a part of the issue [#5633 ](https://github.com/jaegertracing/jaeger/issues/5633) **Description of the changes** This is a PR to achieve Observability Parity in metrics between V1 and V2 components by configuring OTEL Collector to emit desired metrics. **How was this change tested?** The changes were tested by running the following command: ```bash make test ``` ```bash CI actions and compare.py script ``` **Checklist** - [x] I have read [CONTRIBUTING_GUIDELINES.md](https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md) - [x] I have signed all commits - [x] I have added unit tests for the new functionality - [x] I have run lint and test steps successfully - `for jaeger: make lint test` - `for jaeger-ui: yarn lint` and `yarn test` --------- Signed-off-by: Wise-Wizard --- internal/metrics/otelmetrics/factory.go | 9 ++++++++- internal/metrics/otelmetrics/factory_test.go | 11 +++++++++++ scripts/compare_metrics.py | 9 ++------- 3 files changed, 21 insertions(+), 8 deletions(-) diff --git a/internal/metrics/otelmetrics/factory.go b/internal/metrics/otelmetrics/factory.go index d14e60c616c..17adec85a04 100644 --- a/internal/metrics/otelmetrics/factory.go +++ b/internal/metrics/otelmetrics/factory.go @@ -32,7 +32,7 @@ func NewFactory(meterProvider metric.MeterProvider) metrics.Factory { } func (f *otelFactory) Counter(opts metrics.Options) metrics.Counter { - name := f.subScope(opts.Name) + name := CounterNamingConvention(f.subScope(opts.Name)) counter, err := f.meter.Int64Counter(name) if err != nil { log.Printf("Error creating OTEL counter: %v", err) @@ -131,3 +131,10 @@ func attributeSetOption(tags map[string]string) metric.MeasurementOption { } return metric.WithAttributes(attributes...) } + +func CounterNamingConvention(name string) string { + if !strings.HasSuffix(name, "_total") { + name += "_total" + } + return name +} diff --git a/internal/metrics/otelmetrics/factory_test.go b/internal/metrics/otelmetrics/factory_test.go index ade7cbf8064..593e4bb9ade 100644 --- a/internal/metrics/otelmetrics/factory_test.go +++ b/internal/metrics/otelmetrics/factory_test.go @@ -104,6 +104,17 @@ func TestCounter(t *testing.T) { assert.Equal(t, expectedLabels, promLabelsToMap(metrics[0].GetLabel())) } +func TestCounterNamingConvention(t *testing.T) { + input := "test_counter" + expected := "test_counter_total" + + result := otelmetrics.CounterNamingConvention(input) + + if result != expected { + t.Errorf("Expected %s, but got %s", expected, result) + } +} + func TestGauge(t *testing.T) { registry := promReg.NewPedanticRegistry() factory := newTestFactory(t, registry) diff --git a/scripts/compare_metrics.py b/scripts/compare_metrics.py index 8869f1826c6..a34d25556b8 100644 --- a/scripts/compare_metrics.py +++ b/scripts/compare_metrics.py @@ -1,9 +1,3 @@ -# Run the following commands first to create the JSON files: -# Run V1 Binary -# prom2json http://localhost:14269/metrics > V1_Metrics.json -# Run V2 Binary -# prom2json http://localhost:8888/metrics > V2_Metrics.json - import json v1_metrics_path = "./V1_Metrics.json" @@ -41,7 +35,7 @@ def extract_metrics_with_labels(metrics, strip_prefix=None): for name, labels in v1_metrics_with_labels.items(): if name in v2_metrics_with_labels: common_metrics[name] = labels - else: + elif not name.startswith("jaeger_agent"): v1_only_metrics[name] = labels for name, labels in v2_metrics_with_labels.items(): @@ -54,6 +48,7 @@ def extract_metrics_with_labels(metrics, strip_prefix=None): "v2_only_metrics": v2_only_metrics } +# Write the differences to a new JSON file differences_path = "./differences.json" with open(differences_path, 'w') as file: json.dump(differences, file, indent=4) From bcb391942d972db052a6d612938249e529abe373 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Thu, 8 Aug 2024 13:04:10 -0400 Subject: [PATCH 07/12] Bump golang from 1.22.5-alpine to 1.22.6-alpine in /docker/debug (#5816) Bumps golang from 1.22.5-alpine to 1.22.6-alpine. [![Dependabot compatibility score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=golang&package-manager=docker&previous-version=1.22.5-alpine&new-version=1.22.6-alpine)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) ---
Dependabot commands and options
You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot merge` will merge this PR after your CI passes on it - `@dependabot squash and merge` will squash and merge this PR after your CI passes on it - `@dependabot cancel merge` will cancel a previously requested merge and block automerging - `@dependabot reopen` will reopen this PR if it is closed - `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually - `@dependabot show ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)
Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- docker/debug/Dockerfile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docker/debug/Dockerfile b/docker/debug/Dockerfile index 31cf0351e9b..8413e09c72b 100644 --- a/docker/debug/Dockerfile +++ b/docker/debug/Dockerfile @@ -1,4 +1,4 @@ -FROM golang:1.22.5-alpine AS build +FROM golang:1.22.6-alpine AS build ARG TARGETARCH ENV GOPATH /go RUN apk add --update --no-cache ca-certificates make git build-base mailcap @@ -10,7 +10,7 @@ RUN if [[ "$TARGETARCH" == "s390x" || "$TARGETARCH" == "ppc64le" ]] ; then \ go install github.com/go-delve/delve/cmd/dlv@latest; \ fi -FROM golang:1.22.5-alpine +FROM golang:1.22.6-alpine COPY --from=build /go/bin/dlv /go/bin/dlv COPY --from=build /etc/ssl/certs/ca-certificates.crt /etc/ssl/certs/ca-certificates.crt COPY --from=build /etc/mime.types /etc/mime.types From aeb457a492cec84208ee9abc9bc20a8c494cf882 Mon Sep 17 00:00:00 2001 From: Nabil Salah Date: Thu, 8 Aug 2024 21:22:37 +0300 Subject: [PATCH 08/12] Added unit tests in pkg/es/config (#5806) ## Which problem is this PR solving? partially fixes: #5068 ## Description of the changes - This commit adds tests for the `pkg/es/config` package. ## How was this change tested? - `make test` ## Checklist - [x] I have read https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md - [x] I have signed all commits - [x] I have added unit tests for the new functionality - [x] I have run lint and test steps successfully - for `jaeger`: `make lint test` - for `jaeger-ui`: `yarn lint` and `yarn test` Signed-off-by: nabil salah Co-authored-by: Yuri Shkuro --- jaeger-ui | 2 +- pkg/es/config/config_test.go | 625 +++++++++++++++++++++++++++++++++++ 2 files changed, 626 insertions(+), 1 deletion(-) create mode 100644 pkg/es/config/config_test.go diff --git a/jaeger-ui b/jaeger-ui index 3b093f81dec..1704a9a66ae 160000 --- a/jaeger-ui +++ b/jaeger-ui @@ -1 +1 @@ -Subproject commit 3b093f81dec59c6bb04daad96b1b77bd03a29e4a +Subproject commit 1704a9a66ae780ec6bb2f524116176aa9c8a23f8 diff --git a/pkg/es/config/config_test.go b/pkg/es/config/config_test.go new file mode 100644 index 00000000000..285dead5291 --- /dev/null +++ b/pkg/es/config/config_test.go @@ -0,0 +1,625 @@ +// Copyright (c) 2024 The Jaeger Authors. +// SPDX-License-Identifier: Apache-2.0 + +package config + +import ( + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "testing" + "time" + + "github.com/stretchr/testify/require" + "go.uber.org/zap" + + "github.com/jaegertracing/jaeger/pkg/config/tlscfg" + "github.com/jaegertracing/jaeger/pkg/metrics" +) + +var mockEsServerResponseWithVersion0 = []byte(` +{ + "Version": { + "Number": "0" + } +} +`) + +var mockEsServerResponseWithVersion1 = []byte(` +{ + "tagline": "OpenSearch", + "Version": { + "Number": "1" + } +} +`) + +var mockEsServerResponseWithVersion2 = []byte(` +{ + "tagline": "OpenSearch", + "Version": { + "Number": "2" + } +} +`) + +var mockEsServerResponseWithVersion8 = []byte(` +{ + "tagline": "OpenSearch", + "Version": { + "Number": "9" + } +} +`) + +func copyToTempFile(t *testing.T, pattern string, filename string) (file *os.File) { + tempDir := t.TempDir() + tempFilePath := tempDir + "/" + pattern + tempFile, err := os.Create(tempFilePath) + require.NoError(t, err) + data, err := os.ReadFile(filename) + require.NoError(t, err) + + _, err = tempFile.Write(data) + require.NoError(t, err) + require.NoError(t, tempFile.Close()) + + return tempFile +} + +func TestNewClient(t *testing.T) { + const ( + pwd1 = "password" + token = "token" + serverCert = "../../config/tlscfg/testdata/example-server-cert.pem" + ) + pwdFile := filepath.Join(t.TempDir(), "pwd") + require.NoError(t, os.WriteFile(pwdFile, []byte(pwd1), 0o600)) + pwdtokenFile := filepath.Join(t.TempDir(), "token") + require.NoError(t, os.WriteFile(pwdtokenFile, []byte(token), 0o600)) + // copy certs to temp so we can modify them + certFilePath := copyToTempFile(t, "cert.crt", serverCert) + + testServer := httptest.NewServer(http.HandlerFunc(func(res http.ResponseWriter, req *http.Request) { + require.Equal(t, http.MethodGet, req.Method) + res.WriteHeader(http.StatusOK) + res.Write(mockEsServerResponseWithVersion0) + })) + defer testServer.Close() + testServer1 := httptest.NewServer(http.HandlerFunc(func(res http.ResponseWriter, req *http.Request) { + require.Equal(t, http.MethodGet, req.Method) + res.WriteHeader(http.StatusOK) + res.Write(mockEsServerResponseWithVersion1) + })) + defer testServer1.Close() + testServer2 := httptest.NewServer(http.HandlerFunc(func(res http.ResponseWriter, req *http.Request) { + require.Equal(t, http.MethodGet, req.Method) + res.WriteHeader(http.StatusOK) + res.Write(mockEsServerResponseWithVersion2) + })) + defer testServer2.Close() + testServer8 := httptest.NewServer(http.HandlerFunc(func(res http.ResponseWriter, req *http.Request) { + require.Equal(t, http.MethodGet, req.Method) + res.WriteHeader(http.StatusOK) + res.Write(mockEsServerResponseWithVersion8) + })) + defer testServer8.Close() + tests := []struct { + name string + config *Configuration + expectedError bool + }{ + { + name: "success with valid configuration", + config: &Configuration{ + Servers: []string{testServer.URL}, + AllowTokenFromContext: true, + Password: "secret", + LogLevel: "debug", + Username: "user", + PasswordFilePath: "", + BulkSize: -1, // disable bulk; we want immediate flush + Version: 8, + }, + expectedError: false, + }, + { + name: "success with valid configuration and tls enabled", + config: &Configuration{ + Servers: []string{testServer.URL}, + AllowTokenFromContext: true, + Password: "secret", + LogLevel: "debug", + Username: "user", + PasswordFilePath: "", + BulkSize: -1, // disable bulk; we want immediate flush + Version: 8, + TLS: tlscfg.Options{Enabled: true}, + }, + expectedError: false, + }, + { + name: "success with valid configuration and reading token and certicate from file", + config: &Configuration{ + Servers: []string{testServer.URL}, + AllowTokenFromContext: true, + Password: "secret", + LogLevel: "debug", + Username: "user", + PasswordFilePath: "", + BulkSize: -1, // disable bulk; we want immediate flush + Version: 1, + TLS: tlscfg.Options{Enabled: false, CAPath: certFilePath.Name()}, + TokenFilePath: pwdtokenFile, + }, + expectedError: false, + }, + { + name: "succes with invalid configuration of version higher than 8", + config: &Configuration{ + Servers: []string{testServer8.URL}, + AllowTokenFromContext: true, + Password: "secret", + LogLevel: "debug", + Username: "user", + PasswordFilePath: "", + BulkSize: -1, // disable bulk; we want immediate flush + Version: 9, + }, + expectedError: false, + }, + { + name: "success with valid configuration with version 1", + config: &Configuration{ + Servers: []string{testServer1.URL}, + AllowTokenFromContext: true, + Password: "secret", + LogLevel: "debug", + Username: "user", + PasswordFilePath: "", + BulkSize: -1, // disable bulk; we want immediate flush + }, + expectedError: false, + }, + { + name: "success with valid configuration with version 2", + config: &Configuration{ + Servers: []string{testServer2.URL}, + AllowTokenFromContext: true, + Password: "secret", + LogLevel: "debug", + Username: "user", + PasswordFilePath: "", + BulkSize: -1, // disable bulk; we want immediate flush + }, + expectedError: false, + }, + { + name: "success with valid configuration password from file", + config: &Configuration{ + Servers: []string{testServer.URL}, + AllowTokenFromContext: true, + Password: "", + LogLevel: "debug", + Username: "user", + PasswordFilePath: pwdFile, + BulkSize: -1, // disable bulk; we want immediate flush + }, + expectedError: false, + }, + { + name: "fali with configuration password and password from file are set", + config: &Configuration{ + Servers: []string{testServer.URL}, + AllowTokenFromContext: true, + Password: "secret", + LogLevel: "debug", + Username: "user", + PasswordFilePath: pwdFile, + BulkSize: -1, // disable bulk; we want immediate flush + }, + expectedError: true, + }, + { + name: "fail with missing server", + config: &Configuration{ + Servers: []string{}, + AllowTokenFromContext: true, + Password: "secret", + LogLevel: "debug", + Username: "user", + PasswordFilePath: "", + BulkSize: -1, // disable bulk; we want immediate flush + }, + expectedError: true, + }, + { + name: "fail with invalid configuration invalid loglevel", + config: &Configuration{ + Servers: []string{testServer.URL}, + AllowTokenFromContext: true, + Password: "secret", + LogLevel: "invalid", + Username: "user", + PasswordFilePath: "", + BulkSize: -1, // disable bulk; we want immediate flush + }, + expectedError: true, + }, + { + name: "fail with invalid configuration invalid bulkworkers number", + config: &Configuration{ + Servers: []string{testServer.URL}, + AllowTokenFromContext: true, + Password: "secret", + LogLevel: "invalid", + Username: "user", + PasswordFilePath: "", + BulkWorkers: 0, + BulkSize: -1, // disable bulk; we want immediate flush + }, + expectedError: true, + }, + { + name: "success with valid configuration and info log level", + config: &Configuration{ + Servers: []string{testServer.URL}, + AllowTokenFromContext: true, + Password: "secret", + LogLevel: "info", + Username: "user", + PasswordFilePath: "", + BulkSize: -1, // disable bulk; we want immediate flush + }, + expectedError: false, + }, + { + name: "success with valid configuration and error log level", + config: &Configuration{ + Servers: []string{testServer.URL}, + AllowTokenFromContext: true, + Password: "secret", + LogLevel: "error", + Username: "user", + PasswordFilePath: "", + BulkSize: -1, // disable bulk; we want immediate flush + }, + expectedError: false, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + logger := zap.NewNop() + metricsFactory := metrics.NullFactory + config := test.config + client, err := NewClient(config, logger, metricsFactory) + if test.expectedError { + require.Error(t, err) + require.Nil(t, client) + } else { + require.NoError(t, err) + require.NotNil(t, client) + } + }) + } +} + +func TestApplyDefaults(t *testing.T) { + source := &Configuration{ + RemoteReadClusters: []string{"cluster1", "cluster2"}, + Username: "sourceUser", + Password: "sourcePass", + Sniffer: true, + MaxSpanAge: 100, + AdaptiveSamplingLookback: 50, + NumShards: 5, + NumReplicas: 1, + PrioritySpanTemplate: 10, + PriorityServiceTemplate: 20, + PriorityDependenciesTemplate: 30, + BulkSize: 1000, + BulkWorkers: 10, + BulkActions: 100, + BulkFlushInterval: 30, + SnifferTLSEnabled: true, + Tags: TagsAsFields{AllAsFields: true, DotReplacement: "dot", Include: "include", File: "file"}, + MaxDocCount: 10000, + LogLevel: "info", + SendGetBodyAs: "json", + } + + tests := []struct { + name string + target *Configuration + expected *Configuration + }{ + { + name: "All Defaults Applied except PriorityDependenciesTemplate", + target: &Configuration{ + PriorityDependenciesTemplate: 30, + }, // All fields are empty + expected: source, + }, + { + name: "Some Defaults Applied", + target: &Configuration{ + RemoteReadClusters: []string{"customCluster"}, + Username: "customUser", + PrioritySpanTemplate: 10, + PriorityServiceTemplate: 20, + PriorityDependenciesTemplate: 30, + // Other fields left default + }, + expected: &Configuration{ + RemoteReadClusters: []string{"customCluster"}, + Username: "customUser", + Password: "sourcePass", + Sniffer: true, + SnifferTLSEnabled: true, + MaxSpanAge: 100, + AdaptiveSamplingLookback: 50, + NumShards: 5, + NumReplicas: 1, + PrioritySpanTemplate: 10, + PriorityServiceTemplate: 20, + PriorityDependenciesTemplate: 30, + BulkSize: 1000, + BulkWorkers: 10, + BulkActions: 100, + BulkFlushInterval: 30, + Tags: TagsAsFields{AllAsFields: true, DotReplacement: "dot", Include: "include", File: "file"}, + MaxDocCount: 10000, + LogLevel: "info", + SendGetBodyAs: "json", + }, + }, + { + name: "No Defaults Applied", + target: &Configuration{ + RemoteReadClusters: []string{"cluster1", "cluster2"}, + Username: "sourceUser", + Password: "sourcePass", + Sniffer: true, + MaxSpanAge: 100, + AdaptiveSamplingLookback: 50, + NumShards: 5, + NumReplicas: 1, + PrioritySpanTemplate: 10, + PriorityServiceTemplate: 20, + PriorityDependenciesTemplate: 30, + BulkSize: 1000, + BulkWorkers: 10, + BulkActions: 100, + BulkFlushInterval: 30, + SnifferTLSEnabled: true, + Tags: TagsAsFields{AllAsFields: true, DotReplacement: "dot", Include: "include", File: "file"}, + MaxDocCount: 10000, + LogLevel: "info", + SendGetBodyAs: "json", + }, + expected: source, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + test.target.ApplyDefaults(source) + require.Equal(t, test.expected, test.target) + }) + } +} + +func TestTagKeysAsFields(t *testing.T) { + const ( + pwd1 = "tag1\ntag2" + ) + pwdFile := filepath.Join(t.TempDir(), "pwd") + require.NoError(t, os.WriteFile(pwdFile, []byte(pwd1), 0o600)) + + tests := []struct { + name string + config *Configuration + expectedTags []string + expectError bool + }{ + { + name: "File with tags", + config: &Configuration{ + Tags: TagsAsFields{ + File: pwdFile, + Include: "", + }, + }, + expectedTags: []string{"tag1", "tag2"}, + expectError: false, + }, + { + name: "include with tags", + config: &Configuration{ + Tags: TagsAsFields{ + File: "", + Include: "cmdtag1,cmdtag2", + }, + }, + expectedTags: []string{"cmdtag1", "cmdtag2"}, + expectError: false, + }, + { + name: "File and include with tags", + config: &Configuration{ + Tags: TagsAsFields{ + File: pwdFile, + Include: "cmdtag1,cmdtag2", + }, + }, + expectedTags: []string{"tag1", "tag2", "cmdtag1", "cmdtag2"}, + expectError: false, + }, + { + name: "File read error", + config: &Configuration{ + Tags: TagsAsFields{ + File: "/invalid/path/to/file.txt", + Include: "", + }, + }, + expectedTags: nil, + expectError: true, + }, + { + name: "Empty file and params", + config: &Configuration{ + Tags: TagsAsFields{ + File: "", + Include: "", + }, + }, + expectedTags: nil, + expectError: false, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + tags, err := test.config.TagKeysAsFields() + + if test.expectError { + require.Error(t, err) + require.Nil(t, tags) + } else { + require.NoError(t, err) + require.ElementsMatch(t, test.expectedTags, tags) + } + }) + } +} + +func TestGetIndexRolloverFrequencySpansDuration(t *testing.T) { + tests := []struct { + name string + indexFrequency string + expected time.Duration + }{ + { + name: "hourly jaeger-span", + indexFrequency: "hour", + expected: -1 * time.Hour, + }, + { + name: "daily jaeger-span", + indexFrequency: "daily", + expected: -24 * time.Hour, + }, + { + name: "empty jaeger-span", + indexFrequency: "", + expected: -24 * time.Hour, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + c := &Configuration{IndexRolloverFrequencySpans: test.indexFrequency} + got := c.GetIndexRolloverFrequencySpansDuration() + require.Equal(t, test.expected, got) + }) + } +} + +func TestGetIndexRolloverFrequencyServicesDuration(t *testing.T) { + tests := []struct { + name string + indexFrequency string + expected time.Duration + }{ + { + name: "hourly jaeger-service", + indexFrequency: "hour", + expected: -1 * time.Hour, + }, + { + name: "daily jaeger-service", + indexFrequency: "daily", + expected: -24 * time.Hour, + }, + { + name: "empty jaeger-service", + indexFrequency: "", + expected: -24 * time.Hour, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + c := &Configuration{IndexRolloverFrequencyServices: test.indexFrequency} + got := c.GetIndexRolloverFrequencyServicesDuration() + require.Equal(t, test.expected, got) + }) + } +} + +func TestGetIndexRolloverFrequencySamplingDuration(t *testing.T) { + tests := []struct { + name string + indexFrequency string + expected time.Duration + }{ + { + name: "hourly jaeger-sampling", + indexFrequency: "hour", + expected: -1 * time.Hour, + }, + { + name: "daily jaeger-sampling", + indexFrequency: "daily", + expected: -24 * time.Hour, + }, + { + name: "empty jaeger-sampling", + indexFrequency: "", + expected: -24 * time.Hour, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + c := &Configuration{IndexRolloverFrequencySampling: test.indexFrequency} + got := c.GetIndexRolloverFrequencySamplingDuration() + require.Equal(t, test.expected, got) + }) + } +} + +func TestValidate(t *testing.T) { + tests := []struct { + name string + config *Configuration + expectedError bool + }{ + { + name: "All valid input are set", + config: &Configuration{ + Servers: []string{"localhost:8000/dummyserver"}, + }, + expectedError: false, + }, + { + name: "no valid input are set", + config: &Configuration{}, + expectedError: true, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + got := test.config.Validate() + if test.expectedError { + require.Error(t, got) + } else { + require.NoError(t, got) + } + }) + } +} From 0da1609a1755a5a40363208e71adbc5b45f24e8b Mon Sep 17 00:00:00 2001 From: Yuri Shkuro Date: Thu, 8 Aug 2024 14:24:30 -0400 Subject: [PATCH 09/12] Revert "Added unit tests in pkg/es/config (#5806)" This reverts commit aeb457a492cec84208ee9abc9bc20a8c494cf882. The PR incorrectly include UI submodule changes. Signed-off-by: Yuri Shkuro --- jaeger-ui | 2 +- pkg/es/config/config_test.go | 625 ----------------------------------- 2 files changed, 1 insertion(+), 626 deletions(-) delete mode 100644 pkg/es/config/config_test.go diff --git a/jaeger-ui b/jaeger-ui index 1704a9a66ae..3b093f81dec 160000 --- a/jaeger-ui +++ b/jaeger-ui @@ -1 +1 @@ -Subproject commit 1704a9a66ae780ec6bb2f524116176aa9c8a23f8 +Subproject commit 3b093f81dec59c6bb04daad96b1b77bd03a29e4a diff --git a/pkg/es/config/config_test.go b/pkg/es/config/config_test.go deleted file mode 100644 index 285dead5291..00000000000 --- a/pkg/es/config/config_test.go +++ /dev/null @@ -1,625 +0,0 @@ -// Copyright (c) 2024 The Jaeger Authors. -// SPDX-License-Identifier: Apache-2.0 - -package config - -import ( - "net/http" - "net/http/httptest" - "os" - "path/filepath" - "testing" - "time" - - "github.com/stretchr/testify/require" - "go.uber.org/zap" - - "github.com/jaegertracing/jaeger/pkg/config/tlscfg" - "github.com/jaegertracing/jaeger/pkg/metrics" -) - -var mockEsServerResponseWithVersion0 = []byte(` -{ - "Version": { - "Number": "0" - } -} -`) - -var mockEsServerResponseWithVersion1 = []byte(` -{ - "tagline": "OpenSearch", - "Version": { - "Number": "1" - } -} -`) - -var mockEsServerResponseWithVersion2 = []byte(` -{ - "tagline": "OpenSearch", - "Version": { - "Number": "2" - } -} -`) - -var mockEsServerResponseWithVersion8 = []byte(` -{ - "tagline": "OpenSearch", - "Version": { - "Number": "9" - } -} -`) - -func copyToTempFile(t *testing.T, pattern string, filename string) (file *os.File) { - tempDir := t.TempDir() - tempFilePath := tempDir + "/" + pattern - tempFile, err := os.Create(tempFilePath) - require.NoError(t, err) - data, err := os.ReadFile(filename) - require.NoError(t, err) - - _, err = tempFile.Write(data) - require.NoError(t, err) - require.NoError(t, tempFile.Close()) - - return tempFile -} - -func TestNewClient(t *testing.T) { - const ( - pwd1 = "password" - token = "token" - serverCert = "../../config/tlscfg/testdata/example-server-cert.pem" - ) - pwdFile := filepath.Join(t.TempDir(), "pwd") - require.NoError(t, os.WriteFile(pwdFile, []byte(pwd1), 0o600)) - pwdtokenFile := filepath.Join(t.TempDir(), "token") - require.NoError(t, os.WriteFile(pwdtokenFile, []byte(token), 0o600)) - // copy certs to temp so we can modify them - certFilePath := copyToTempFile(t, "cert.crt", serverCert) - - testServer := httptest.NewServer(http.HandlerFunc(func(res http.ResponseWriter, req *http.Request) { - require.Equal(t, http.MethodGet, req.Method) - res.WriteHeader(http.StatusOK) - res.Write(mockEsServerResponseWithVersion0) - })) - defer testServer.Close() - testServer1 := httptest.NewServer(http.HandlerFunc(func(res http.ResponseWriter, req *http.Request) { - require.Equal(t, http.MethodGet, req.Method) - res.WriteHeader(http.StatusOK) - res.Write(mockEsServerResponseWithVersion1) - })) - defer testServer1.Close() - testServer2 := httptest.NewServer(http.HandlerFunc(func(res http.ResponseWriter, req *http.Request) { - require.Equal(t, http.MethodGet, req.Method) - res.WriteHeader(http.StatusOK) - res.Write(mockEsServerResponseWithVersion2) - })) - defer testServer2.Close() - testServer8 := httptest.NewServer(http.HandlerFunc(func(res http.ResponseWriter, req *http.Request) { - require.Equal(t, http.MethodGet, req.Method) - res.WriteHeader(http.StatusOK) - res.Write(mockEsServerResponseWithVersion8) - })) - defer testServer8.Close() - tests := []struct { - name string - config *Configuration - expectedError bool - }{ - { - name: "success with valid configuration", - config: &Configuration{ - Servers: []string{testServer.URL}, - AllowTokenFromContext: true, - Password: "secret", - LogLevel: "debug", - Username: "user", - PasswordFilePath: "", - BulkSize: -1, // disable bulk; we want immediate flush - Version: 8, - }, - expectedError: false, - }, - { - name: "success with valid configuration and tls enabled", - config: &Configuration{ - Servers: []string{testServer.URL}, - AllowTokenFromContext: true, - Password: "secret", - LogLevel: "debug", - Username: "user", - PasswordFilePath: "", - BulkSize: -1, // disable bulk; we want immediate flush - Version: 8, - TLS: tlscfg.Options{Enabled: true}, - }, - expectedError: false, - }, - { - name: "success with valid configuration and reading token and certicate from file", - config: &Configuration{ - Servers: []string{testServer.URL}, - AllowTokenFromContext: true, - Password: "secret", - LogLevel: "debug", - Username: "user", - PasswordFilePath: "", - BulkSize: -1, // disable bulk; we want immediate flush - Version: 1, - TLS: tlscfg.Options{Enabled: false, CAPath: certFilePath.Name()}, - TokenFilePath: pwdtokenFile, - }, - expectedError: false, - }, - { - name: "succes with invalid configuration of version higher than 8", - config: &Configuration{ - Servers: []string{testServer8.URL}, - AllowTokenFromContext: true, - Password: "secret", - LogLevel: "debug", - Username: "user", - PasswordFilePath: "", - BulkSize: -1, // disable bulk; we want immediate flush - Version: 9, - }, - expectedError: false, - }, - { - name: "success with valid configuration with version 1", - config: &Configuration{ - Servers: []string{testServer1.URL}, - AllowTokenFromContext: true, - Password: "secret", - LogLevel: "debug", - Username: "user", - PasswordFilePath: "", - BulkSize: -1, // disable bulk; we want immediate flush - }, - expectedError: false, - }, - { - name: "success with valid configuration with version 2", - config: &Configuration{ - Servers: []string{testServer2.URL}, - AllowTokenFromContext: true, - Password: "secret", - LogLevel: "debug", - Username: "user", - PasswordFilePath: "", - BulkSize: -1, // disable bulk; we want immediate flush - }, - expectedError: false, - }, - { - name: "success with valid configuration password from file", - config: &Configuration{ - Servers: []string{testServer.URL}, - AllowTokenFromContext: true, - Password: "", - LogLevel: "debug", - Username: "user", - PasswordFilePath: pwdFile, - BulkSize: -1, // disable bulk; we want immediate flush - }, - expectedError: false, - }, - { - name: "fali with configuration password and password from file are set", - config: &Configuration{ - Servers: []string{testServer.URL}, - AllowTokenFromContext: true, - Password: "secret", - LogLevel: "debug", - Username: "user", - PasswordFilePath: pwdFile, - BulkSize: -1, // disable bulk; we want immediate flush - }, - expectedError: true, - }, - { - name: "fail with missing server", - config: &Configuration{ - Servers: []string{}, - AllowTokenFromContext: true, - Password: "secret", - LogLevel: "debug", - Username: "user", - PasswordFilePath: "", - BulkSize: -1, // disable bulk; we want immediate flush - }, - expectedError: true, - }, - { - name: "fail with invalid configuration invalid loglevel", - config: &Configuration{ - Servers: []string{testServer.URL}, - AllowTokenFromContext: true, - Password: "secret", - LogLevel: "invalid", - Username: "user", - PasswordFilePath: "", - BulkSize: -1, // disable bulk; we want immediate flush - }, - expectedError: true, - }, - { - name: "fail with invalid configuration invalid bulkworkers number", - config: &Configuration{ - Servers: []string{testServer.URL}, - AllowTokenFromContext: true, - Password: "secret", - LogLevel: "invalid", - Username: "user", - PasswordFilePath: "", - BulkWorkers: 0, - BulkSize: -1, // disable bulk; we want immediate flush - }, - expectedError: true, - }, - { - name: "success with valid configuration and info log level", - config: &Configuration{ - Servers: []string{testServer.URL}, - AllowTokenFromContext: true, - Password: "secret", - LogLevel: "info", - Username: "user", - PasswordFilePath: "", - BulkSize: -1, // disable bulk; we want immediate flush - }, - expectedError: false, - }, - { - name: "success with valid configuration and error log level", - config: &Configuration{ - Servers: []string{testServer.URL}, - AllowTokenFromContext: true, - Password: "secret", - LogLevel: "error", - Username: "user", - PasswordFilePath: "", - BulkSize: -1, // disable bulk; we want immediate flush - }, - expectedError: false, - }, - } - - for _, test := range tests { - t.Run(test.name, func(t *testing.T) { - logger := zap.NewNop() - metricsFactory := metrics.NullFactory - config := test.config - client, err := NewClient(config, logger, metricsFactory) - if test.expectedError { - require.Error(t, err) - require.Nil(t, client) - } else { - require.NoError(t, err) - require.NotNil(t, client) - } - }) - } -} - -func TestApplyDefaults(t *testing.T) { - source := &Configuration{ - RemoteReadClusters: []string{"cluster1", "cluster2"}, - Username: "sourceUser", - Password: "sourcePass", - Sniffer: true, - MaxSpanAge: 100, - AdaptiveSamplingLookback: 50, - NumShards: 5, - NumReplicas: 1, - PrioritySpanTemplate: 10, - PriorityServiceTemplate: 20, - PriorityDependenciesTemplate: 30, - BulkSize: 1000, - BulkWorkers: 10, - BulkActions: 100, - BulkFlushInterval: 30, - SnifferTLSEnabled: true, - Tags: TagsAsFields{AllAsFields: true, DotReplacement: "dot", Include: "include", File: "file"}, - MaxDocCount: 10000, - LogLevel: "info", - SendGetBodyAs: "json", - } - - tests := []struct { - name string - target *Configuration - expected *Configuration - }{ - { - name: "All Defaults Applied except PriorityDependenciesTemplate", - target: &Configuration{ - PriorityDependenciesTemplate: 30, - }, // All fields are empty - expected: source, - }, - { - name: "Some Defaults Applied", - target: &Configuration{ - RemoteReadClusters: []string{"customCluster"}, - Username: "customUser", - PrioritySpanTemplate: 10, - PriorityServiceTemplate: 20, - PriorityDependenciesTemplate: 30, - // Other fields left default - }, - expected: &Configuration{ - RemoteReadClusters: []string{"customCluster"}, - Username: "customUser", - Password: "sourcePass", - Sniffer: true, - SnifferTLSEnabled: true, - MaxSpanAge: 100, - AdaptiveSamplingLookback: 50, - NumShards: 5, - NumReplicas: 1, - PrioritySpanTemplate: 10, - PriorityServiceTemplate: 20, - PriorityDependenciesTemplate: 30, - BulkSize: 1000, - BulkWorkers: 10, - BulkActions: 100, - BulkFlushInterval: 30, - Tags: TagsAsFields{AllAsFields: true, DotReplacement: "dot", Include: "include", File: "file"}, - MaxDocCount: 10000, - LogLevel: "info", - SendGetBodyAs: "json", - }, - }, - { - name: "No Defaults Applied", - target: &Configuration{ - RemoteReadClusters: []string{"cluster1", "cluster2"}, - Username: "sourceUser", - Password: "sourcePass", - Sniffer: true, - MaxSpanAge: 100, - AdaptiveSamplingLookback: 50, - NumShards: 5, - NumReplicas: 1, - PrioritySpanTemplate: 10, - PriorityServiceTemplate: 20, - PriorityDependenciesTemplate: 30, - BulkSize: 1000, - BulkWorkers: 10, - BulkActions: 100, - BulkFlushInterval: 30, - SnifferTLSEnabled: true, - Tags: TagsAsFields{AllAsFields: true, DotReplacement: "dot", Include: "include", File: "file"}, - MaxDocCount: 10000, - LogLevel: "info", - SendGetBodyAs: "json", - }, - expected: source, - }, - } - - for _, test := range tests { - t.Run(test.name, func(t *testing.T) { - test.target.ApplyDefaults(source) - require.Equal(t, test.expected, test.target) - }) - } -} - -func TestTagKeysAsFields(t *testing.T) { - const ( - pwd1 = "tag1\ntag2" - ) - pwdFile := filepath.Join(t.TempDir(), "pwd") - require.NoError(t, os.WriteFile(pwdFile, []byte(pwd1), 0o600)) - - tests := []struct { - name string - config *Configuration - expectedTags []string - expectError bool - }{ - { - name: "File with tags", - config: &Configuration{ - Tags: TagsAsFields{ - File: pwdFile, - Include: "", - }, - }, - expectedTags: []string{"tag1", "tag2"}, - expectError: false, - }, - { - name: "include with tags", - config: &Configuration{ - Tags: TagsAsFields{ - File: "", - Include: "cmdtag1,cmdtag2", - }, - }, - expectedTags: []string{"cmdtag1", "cmdtag2"}, - expectError: false, - }, - { - name: "File and include with tags", - config: &Configuration{ - Tags: TagsAsFields{ - File: pwdFile, - Include: "cmdtag1,cmdtag2", - }, - }, - expectedTags: []string{"tag1", "tag2", "cmdtag1", "cmdtag2"}, - expectError: false, - }, - { - name: "File read error", - config: &Configuration{ - Tags: TagsAsFields{ - File: "/invalid/path/to/file.txt", - Include: "", - }, - }, - expectedTags: nil, - expectError: true, - }, - { - name: "Empty file and params", - config: &Configuration{ - Tags: TagsAsFields{ - File: "", - Include: "", - }, - }, - expectedTags: nil, - expectError: false, - }, - } - - for _, test := range tests { - t.Run(test.name, func(t *testing.T) { - tags, err := test.config.TagKeysAsFields() - - if test.expectError { - require.Error(t, err) - require.Nil(t, tags) - } else { - require.NoError(t, err) - require.ElementsMatch(t, test.expectedTags, tags) - } - }) - } -} - -func TestGetIndexRolloverFrequencySpansDuration(t *testing.T) { - tests := []struct { - name string - indexFrequency string - expected time.Duration - }{ - { - name: "hourly jaeger-span", - indexFrequency: "hour", - expected: -1 * time.Hour, - }, - { - name: "daily jaeger-span", - indexFrequency: "daily", - expected: -24 * time.Hour, - }, - { - name: "empty jaeger-span", - indexFrequency: "", - expected: -24 * time.Hour, - }, - } - - for _, test := range tests { - t.Run(test.name, func(t *testing.T) { - c := &Configuration{IndexRolloverFrequencySpans: test.indexFrequency} - got := c.GetIndexRolloverFrequencySpansDuration() - require.Equal(t, test.expected, got) - }) - } -} - -func TestGetIndexRolloverFrequencyServicesDuration(t *testing.T) { - tests := []struct { - name string - indexFrequency string - expected time.Duration - }{ - { - name: "hourly jaeger-service", - indexFrequency: "hour", - expected: -1 * time.Hour, - }, - { - name: "daily jaeger-service", - indexFrequency: "daily", - expected: -24 * time.Hour, - }, - { - name: "empty jaeger-service", - indexFrequency: "", - expected: -24 * time.Hour, - }, - } - - for _, test := range tests { - t.Run(test.name, func(t *testing.T) { - c := &Configuration{IndexRolloverFrequencyServices: test.indexFrequency} - got := c.GetIndexRolloverFrequencyServicesDuration() - require.Equal(t, test.expected, got) - }) - } -} - -func TestGetIndexRolloverFrequencySamplingDuration(t *testing.T) { - tests := []struct { - name string - indexFrequency string - expected time.Duration - }{ - { - name: "hourly jaeger-sampling", - indexFrequency: "hour", - expected: -1 * time.Hour, - }, - { - name: "daily jaeger-sampling", - indexFrequency: "daily", - expected: -24 * time.Hour, - }, - { - name: "empty jaeger-sampling", - indexFrequency: "", - expected: -24 * time.Hour, - }, - } - - for _, test := range tests { - t.Run(test.name, func(t *testing.T) { - c := &Configuration{IndexRolloverFrequencySampling: test.indexFrequency} - got := c.GetIndexRolloverFrequencySamplingDuration() - require.Equal(t, test.expected, got) - }) - } -} - -func TestValidate(t *testing.T) { - tests := []struct { - name string - config *Configuration - expectedError bool - }{ - { - name: "All valid input are set", - config: &Configuration{ - Servers: []string{"localhost:8000/dummyserver"}, - }, - expectedError: false, - }, - { - name: "no valid input are set", - config: &Configuration{}, - expectedError: true, - }, - } - - for _, test := range tests { - t.Run(test.name, func(t *testing.T) { - got := test.config.Validate() - if test.expectedError { - require.Error(t, got) - } else { - require.NoError(t, got) - } - }) - } -} From 4220d62cf3e2f18356c8504666ff43a1d838c241 Mon Sep 17 00:00:00 2001 From: Jonah Kowall Date: Thu, 8 Aug 2024 16:03:33 -0400 Subject: [PATCH 10/12] Clarify release docs slightly (#5809) Just improving the docs a bit to make sure the release manager does the Jaeger UI before the rest of the directions. Signed-off-by: Jonah Kowall --- RELEASE.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/RELEASE.md b/RELEASE.md index ebf2f3353db..daf1a6688d1 100644 --- a/RELEASE.md +++ b/RELEASE.md @@ -5,7 +5,8 @@ * A curated list of notable changes and links to PRs. Do not simply dump git log, select the changes that affect the users. To obtain the list of all changes run `make changelog` or use `scripts/release-notes.py`. * The section can be split into sub-section if necessary, e.g. UI Changes, Backend Changes, Bug Fixes, etc. - * If the jaeger-ui submodule has changes cut a new release and also upgrade the submodule versions then commit, for example: + * If the jaeger-ui submodule has changes cut a new release + * Then upgrade the submodule versions and finally commit. For example: ``` git submodule init git submodule update From f250716cc0a1131dba46986ac4455eb43c364a15 Mon Sep 17 00:00:00 2001 From: Nabil Salah Date: Fri, 9 Aug 2024 00:52:34 +0300 Subject: [PATCH 11/12] Added unit tests in pkg/es/config (#5819) ## Which problem is this PR solving? Part of: #5068 ## Description of the changes - This commit adds tests for the `pkg/es/config` package. ## How was this change tested? - `make test` ## Checklist - [x] I have read https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md - [x] I have signed all commits - [x] I have added unit tests for the new functionality - [x] I have run lint and test steps successfully - for `jaeger`: `make lint test` - for `jaeger-ui`: `yarn lint` and `yarn test` Signed-off-by: nabil salah --- pkg/es/config/.nocover | 1 - pkg/es/config/config_test.go | 636 +++++++++++++++++++++++++++++++++++ 2 files changed, 636 insertions(+), 1 deletion(-) delete mode 100644 pkg/es/config/.nocover create mode 100644 pkg/es/config/config_test.go diff --git a/pkg/es/config/.nocover b/pkg/es/config/.nocover deleted file mode 100644 index 517b145dea2..00000000000 --- a/pkg/es/config/.nocover +++ /dev/null @@ -1 +0,0 @@ -requires connection to Elasticsearch diff --git a/pkg/es/config/config_test.go b/pkg/es/config/config_test.go new file mode 100644 index 00000000000..7ba124bfd9a --- /dev/null +++ b/pkg/es/config/config_test.go @@ -0,0 +1,636 @@ +// Copyright (c) 2024 The Jaeger Authors. +// SPDX-License-Identifier: Apache-2.0 + +package config + +import ( + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "testing" + "time" + + "github.com/stretchr/testify/require" + "go.uber.org/zap" + + "github.com/jaegertracing/jaeger/pkg/config/tlscfg" + "github.com/jaegertracing/jaeger/pkg/metrics" + "github.com/jaegertracing/jaeger/pkg/testutils" +) + +var mockEsServerResponseWithVersion0 = []byte(` +{ + "Version": { + "Number": "0" + } +} +`) + +var mockEsServerResponseWithVersion1 = []byte(` +{ + "tagline": "OpenSearch", + "Version": { + "Number": "1" + } +} +`) + +var mockEsServerResponseWithVersion2 = []byte(` +{ + "tagline": "OpenSearch", + "Version": { + "Number": "2" + } +} +`) + +var mockEsServerResponseWithVersion8 = []byte(` +{ + "tagline": "OpenSearch", + "Version": { + "Number": "9" + } +} +`) + +func copyToTempFile(t *testing.T, pattern string, filename string) (file *os.File) { + tempDir := t.TempDir() + tempFilePath := tempDir + "/" + pattern + tempFile, err := os.Create(tempFilePath) + require.NoError(t, err) + data, err := os.ReadFile(filename) + + require.NoError(t, err) + + _, err = tempFile.Write(data) + require.NoError(t, err) + require.NoError(t, tempFile.Close()) + + return tempFile +} + +func TestNewClient(t *testing.T) { + const ( + pwd1 = "password" + token = "token" + serverCert = "../../config/tlscfg/testdata/example-server-cert.pem" + ) + pwdFile := filepath.Join(t.TempDir(), "pwd") + require.NoError(t, os.WriteFile(pwdFile, []byte(pwd1), 0o600)) + pwdtokenFile := filepath.Join(t.TempDir(), "token") + require.NoError(t, os.WriteFile(pwdtokenFile, []byte(token), 0o600)) + // copy certs to temp so we can modify them + certFilePath := copyToTempFile(t, "cert.crt", serverCert) + defer certFilePath.Close() + + testServer := httptest.NewServer(http.HandlerFunc(func(res http.ResponseWriter, req *http.Request) { + require.Equal(t, http.MethodGet, req.Method) + res.WriteHeader(http.StatusOK) + res.Write(mockEsServerResponseWithVersion0) + })) + defer testServer.Close() + testServer1 := httptest.NewServer(http.HandlerFunc(func(res http.ResponseWriter, req *http.Request) { + require.Equal(t, http.MethodGet, req.Method) + res.WriteHeader(http.StatusOK) + res.Write(mockEsServerResponseWithVersion1) + })) + defer testServer1.Close() + testServer2 := httptest.NewServer(http.HandlerFunc(func(res http.ResponseWriter, req *http.Request) { + require.Equal(t, http.MethodGet, req.Method) + res.WriteHeader(http.StatusOK) + res.Write(mockEsServerResponseWithVersion2) + })) + defer testServer2.Close() + testServer8 := httptest.NewServer(http.HandlerFunc(func(res http.ResponseWriter, req *http.Request) { + require.Equal(t, http.MethodGet, req.Method) + res.WriteHeader(http.StatusOK) + res.Write(mockEsServerResponseWithVersion8) + })) + defer testServer8.Close() + tests := []struct { + name string + config *Configuration + expectedError bool + }{ + { + name: "success with valid configuration", + config: &Configuration{ + Servers: []string{testServer.URL}, + AllowTokenFromContext: true, + Password: "secret", + LogLevel: "debug", + Username: "user", + PasswordFilePath: "", + BulkSize: -1, // disable bulk; we want immediate flush + Version: 8, + }, + expectedError: false, + }, + { + name: "success with valid configuration and tls enabled", + config: &Configuration{ + Servers: []string{testServer.URL}, + AllowTokenFromContext: true, + Password: "secret", + LogLevel: "debug", + Username: "user", + PasswordFilePath: "", + BulkSize: -1, // disable bulk; we want immediate flush + Version: 0, + TLS: tlscfg.Options{Enabled: true}, + }, + expectedError: false, + }, + { + name: "success with valid configuration and reading token and certicate from file", + config: &Configuration{ + Servers: []string{testServer.URL}, + AllowTokenFromContext: true, + Password: "secret", + LogLevel: "debug", + Username: "user", + PasswordFilePath: "", + BulkSize: -1, // disable bulk; we want immediate flush + Version: 0, + TLS: tlscfg.Options{Enabled: false, CAPath: certFilePath.Name()}, + TokenFilePath: pwdtokenFile, + }, + expectedError: false, + }, + { + name: "succes with invalid configuration of version higher than 8", + config: &Configuration{ + Servers: []string{testServer8.URL}, + AllowTokenFromContext: true, + Password: "secret", + LogLevel: "debug", + Username: "user", + PasswordFilePath: "", + BulkSize: -1, // disable bulk; we want immediate flush + Version: 9, + }, + expectedError: false, + }, + { + name: "success with valid configuration with version 1", + config: &Configuration{ + Servers: []string{testServer1.URL}, + AllowTokenFromContext: true, + Password: "secret", + LogLevel: "debug", + Username: "user", + PasswordFilePath: "", + BulkSize: -1, // disable bulk; we want immediate flush + }, + expectedError: false, + }, + { + name: "success with valid configuration with version 2", + config: &Configuration{ + Servers: []string{testServer2.URL}, + AllowTokenFromContext: true, + Password: "secret", + LogLevel: "debug", + Username: "user", + PasswordFilePath: "", + BulkSize: -1, // disable bulk; we want immediate flush + }, + expectedError: false, + }, + { + name: "success with valid configuration password from file", + config: &Configuration{ + Servers: []string{testServer.URL}, + AllowTokenFromContext: true, + Password: "", + LogLevel: "debug", + Username: "user", + PasswordFilePath: pwdFile, + BulkSize: -1, // disable bulk; we want immediate flush + }, + expectedError: false, + }, + { + name: "fali with configuration password and password from file are set", + config: &Configuration{ + Servers: []string{testServer.URL}, + AllowTokenFromContext: true, + Password: "secret", + LogLevel: "debug", + Username: "user", + PasswordFilePath: pwdFile, + BulkSize: -1, // disable bulk; we want immediate flush + }, + expectedError: true, + }, + { + name: "fail with missing server", + config: &Configuration{ + Servers: []string{}, + AllowTokenFromContext: true, + Password: "secret", + LogLevel: "debug", + Username: "user", + PasswordFilePath: "", + BulkSize: -1, // disable bulk; we want immediate flush + }, + expectedError: true, + }, + { + name: "fail with invalid configuration invalid loglevel", + config: &Configuration{ + Servers: []string{testServer.URL}, + AllowTokenFromContext: true, + Password: "secret", + LogLevel: "invalid", + Username: "user", + PasswordFilePath: "", + BulkSize: -1, // disable bulk; we want immediate flush + }, + expectedError: true, + }, + { + name: "fail with invalid configuration invalid bulkworkers number", + config: &Configuration{ + Servers: []string{testServer.URL}, + AllowTokenFromContext: true, + Password: "secret", + LogLevel: "invalid", + Username: "user", + PasswordFilePath: "", + BulkWorkers: 0, + BulkSize: -1, // disable bulk; we want immediate flush + }, + expectedError: true, + }, + { + name: "success with valid configuration and info log level", + config: &Configuration{ + Servers: []string{testServer.URL}, + AllowTokenFromContext: true, + Password: "secret", + LogLevel: "info", + Username: "user", + PasswordFilePath: "", + BulkSize: -1, // disable bulk; we want immediate flush + }, + expectedError: false, + }, + { + name: "success with valid configuration and error log level", + config: &Configuration{ + Servers: []string{testServer.URL}, + AllowTokenFromContext: true, + Password: "secret", + LogLevel: "error", + Username: "user", + PasswordFilePath: "", + BulkSize: -1, // disable bulk; we want immediate flush + }, + expectedError: false, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + logger := zap.NewNop() + metricsFactory := metrics.NullFactory + config := test.config + client, err := NewClient(config, logger, metricsFactory) + if test.expectedError { + require.Error(t, err) + require.Nil(t, client) + } else { + require.NoError(t, err) + require.NotNil(t, client) + err = client.Close() + require.NoError(t, err) + } + err = config.TLS.Close() + require.NoError(t, err) + }) + } +} + +func TestApplyDefaults(t *testing.T) { + source := &Configuration{ + RemoteReadClusters: []string{"cluster1", "cluster2"}, + Username: "sourceUser", + Password: "sourcePass", + Sniffer: true, + MaxSpanAge: 100, + AdaptiveSamplingLookback: 50, + NumShards: 5, + NumReplicas: 1, + PrioritySpanTemplate: 10, + PriorityServiceTemplate: 20, + PriorityDependenciesTemplate: 30, + BulkSize: 1000, + BulkWorkers: 10, + BulkActions: 100, + BulkFlushInterval: 30, + SnifferTLSEnabled: true, + Tags: TagsAsFields{AllAsFields: true, DotReplacement: "dot", Include: "include", File: "file"}, + MaxDocCount: 10000, + LogLevel: "info", + SendGetBodyAs: "json", + } + + tests := []struct { + name string + target *Configuration + expected *Configuration + }{ + { + name: "All Defaults Applied except PriorityDependenciesTemplate", + target: &Configuration{ + PriorityDependenciesTemplate: 30, + }, // All fields are empty + expected: source, + }, + { + name: "Some Defaults Applied", + target: &Configuration{ + RemoteReadClusters: []string{"customCluster"}, + Username: "customUser", + PrioritySpanTemplate: 10, + PriorityServiceTemplate: 20, + PriorityDependenciesTemplate: 30, + // Other fields left default + }, + expected: &Configuration{ + RemoteReadClusters: []string{"customCluster"}, + Username: "customUser", + Password: "sourcePass", + Sniffer: true, + SnifferTLSEnabled: true, + MaxSpanAge: 100, + AdaptiveSamplingLookback: 50, + NumShards: 5, + NumReplicas: 1, + PrioritySpanTemplate: 10, + PriorityServiceTemplate: 20, + PriorityDependenciesTemplate: 30, + BulkSize: 1000, + BulkWorkers: 10, + BulkActions: 100, + BulkFlushInterval: 30, + Tags: TagsAsFields{AllAsFields: true, DotReplacement: "dot", Include: "include", File: "file"}, + MaxDocCount: 10000, + LogLevel: "info", + SendGetBodyAs: "json", + }, + }, + { + name: "No Defaults Applied", + target: &Configuration{ + RemoteReadClusters: []string{"cluster1", "cluster2"}, + Username: "sourceUser", + Password: "sourcePass", + Sniffer: true, + MaxSpanAge: 100, + AdaptiveSamplingLookback: 50, + NumShards: 5, + NumReplicas: 1, + PrioritySpanTemplate: 10, + PriorityServiceTemplate: 20, + PriorityDependenciesTemplate: 30, + BulkSize: 1000, + BulkWorkers: 10, + BulkActions: 100, + BulkFlushInterval: 30, + SnifferTLSEnabled: true, + Tags: TagsAsFields{AllAsFields: true, DotReplacement: "dot", Include: "include", File: "file"}, + MaxDocCount: 10000, + LogLevel: "info", + SendGetBodyAs: "json", + }, + expected: source, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + test.target.ApplyDefaults(source) + require.Equal(t, test.expected, test.target) + }) + } +} + +func TestTagKeysAsFields(t *testing.T) { + const ( + pwd1 = "tag1\ntag2" + ) + pwdFile := filepath.Join(t.TempDir(), "pwd") + require.NoError(t, os.WriteFile(pwdFile, []byte(pwd1), 0o600)) + + tests := []struct { + name string + config *Configuration + expectedTags []string + expectError bool + }{ + { + name: "File with tags", + config: &Configuration{ + Tags: TagsAsFields{ + File: pwdFile, + Include: "", + }, + }, + expectedTags: []string{"tag1", "tag2"}, + expectError: false, + }, + { + name: "include with tags", + config: &Configuration{ + Tags: TagsAsFields{ + File: "", + Include: "cmdtag1,cmdtag2", + }, + }, + expectedTags: []string{"cmdtag1", "cmdtag2"}, + expectError: false, + }, + { + name: "File and include with tags", + config: &Configuration{ + Tags: TagsAsFields{ + File: pwdFile, + Include: "cmdtag1,cmdtag2", + }, + }, + expectedTags: []string{"tag1", "tag2", "cmdtag1", "cmdtag2"}, + expectError: false, + }, + { + name: "File read error", + config: &Configuration{ + Tags: TagsAsFields{ + File: "/invalid/path/to/file.txt", + Include: "", + }, + }, + expectedTags: nil, + expectError: true, + }, + { + name: "Empty file and params", + config: &Configuration{ + Tags: TagsAsFields{ + File: "", + Include: "", + }, + }, + expectedTags: nil, + expectError: false, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + tags, err := test.config.TagKeysAsFields() + + if test.expectError { + require.Error(t, err) + require.Nil(t, tags) + } else { + require.NoError(t, err) + require.ElementsMatch(t, test.expectedTags, tags) + } + }) + } +} + +func TestGetIndexRolloverFrequencySpansDuration(t *testing.T) { + tests := []struct { + name string + indexFrequency string + expected time.Duration + }{ + { + name: "hourly jaeger-span", + indexFrequency: "hour", + expected: -1 * time.Hour, + }, + { + name: "daily jaeger-span", + indexFrequency: "daily", + expected: -24 * time.Hour, + }, + { + name: "empty jaeger-span", + indexFrequency: "", + expected: -24 * time.Hour, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + c := &Configuration{IndexRolloverFrequencySpans: test.indexFrequency} + got := c.GetIndexRolloverFrequencySpansDuration() + require.Equal(t, test.expected, got) + }) + } +} + +func TestGetIndexRolloverFrequencyServicesDuration(t *testing.T) { + tests := []struct { + name string + indexFrequency string + expected time.Duration + }{ + { + name: "hourly jaeger-service", + indexFrequency: "hour", + expected: -1 * time.Hour, + }, + { + name: "daily jaeger-service", + indexFrequency: "daily", + expected: -24 * time.Hour, + }, + { + name: "empty jaeger-service", + indexFrequency: "", + expected: -24 * time.Hour, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + c := &Configuration{IndexRolloverFrequencyServices: test.indexFrequency} + got := c.GetIndexRolloverFrequencyServicesDuration() + require.Equal(t, test.expected, got) + }) + } +} + +func TestGetIndexRolloverFrequencySamplingDuration(t *testing.T) { + tests := []struct { + name string + indexFrequency string + expected time.Duration + }{ + { + name: "hourly jaeger-sampling", + indexFrequency: "hour", + expected: -1 * time.Hour, + }, + { + name: "daily jaeger-sampling", + indexFrequency: "daily", + expected: -24 * time.Hour, + }, + { + name: "empty jaeger-sampling", + indexFrequency: "", + expected: -24 * time.Hour, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + c := &Configuration{IndexRolloverFrequencySampling: test.indexFrequency} + got := c.GetIndexRolloverFrequencySamplingDuration() + require.Equal(t, test.expected, got) + }) + } +} + +func TestValidate(t *testing.T) { + tests := []struct { + name string + config *Configuration + expectedError bool + }{ + { + name: "All valid input are set", + config: &Configuration{ + Servers: []string{"localhost:8000/dummyserver"}, + }, + expectedError: false, + }, + { + name: "no valid input are set", + config: &Configuration{}, + expectedError: true, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + got := test.config.Validate() + if test.expectedError { + require.Error(t, got) + } else { + require.NoError(t, got) + } + }) + } +} + +func TestMain(m *testing.M) { + testutils.VerifyGoLeaks(m) +} From a6ccf56ca0b67d1088b7dad3c58e9212c8c66e58 Mon Sep 17 00:00:00 2001 From: Yuri Shkuro Date: Fri, 9 Aug 2024 01:05:43 -0300 Subject: [PATCH 12/12] Clearer output from lint scripts (#5820) --- scripts/check-goleak-files.sh | 11 +++++++---- scripts/check-test-files.sh | 4 ++++ 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/scripts/check-goleak-files.sh b/scripts/check-goleak-files.sh index 47d61295a01..e15fc790e57 100755 --- a/scripts/check-goleak-files.sh +++ b/scripts/check-goleak-files.sh @@ -3,10 +3,12 @@ set -euo pipefail bad_pkgs=0 +total_pkgs=0 failed_pkgs=0 # shellcheck disable=SC2048 for dir in $*; do + ((total_pkgs+=1)) if [[ -f "${dir}/.nocover" ]]; then continue fi @@ -22,10 +24,10 @@ for dir in $*; do fi done if ((good == 0)); then - echo "🔴 Error(check-goleak): no goleak check in package ${dir}" + echo "Error(check-goleak): no goleak check in package ${dir}" ((bad_pkgs+=1)) if [[ "${dir}" == "./cmd/jaeger/internal/integration/" || "${dir}" == "./plugin/storage/integration/" ]]; then - echo " this package is temporarily allowed and will not cause linter failure" + echo " ... this package is temporarily allowed and will not cause linter failure" else ((failed_pkgs+=1)) fi @@ -33,8 +35,7 @@ for dir in $*; do done function help() { - echo " See https://github.com/jaegertracing/jaeger/pull/5010/files" - echo " for examples of adding the checks." + echo " See pkg/version/package_test.go as example of adding the checks." } if ((failed_pkgs > 0)); then @@ -44,4 +45,6 @@ if ((failed_pkgs > 0)); then elif ((bad_pkgs > 0)); then echo "🐞 Warning(check-goleak): no goleak check in ${bad_pkgs} package(s)." help +else + echo "✅ Info(check-goleak): no issues after scanning ${total_pkgs} package(s)." fi diff --git a/scripts/check-test-files.sh b/scripts/check-test-files.sh index 7ffc896688c..8e7386501e7 100755 --- a/scripts/check-test-files.sh +++ b/scripts/check-test-files.sh @@ -9,9 +9,11 @@ set -euo pipefail NO_TEST_FILE_DIRS="" +total_pkgs=0 # shellcheck disable=SC2048 for dir in $*; do + ((total_pkgs+=1)) mainFile=$(find "${dir}" -maxdepth 1 -name 'main.go') testFiles=$(find "${dir}" -maxdepth 1 -name '*_test.go') if [ -z "${testFiles}" ]; then @@ -42,4 +44,6 @@ if [ -n "${NO_TEST_FILE_DIRS}" ]; then echo "error: at least one *_test.go file must be in all directories with go files so that they are counted for code coverage" >&2 echo " if no tests are possible for a package (e.g. it only defines types), create empty_test.go" >&2 exit 1 +else + echo "✅ Info(check-test-files): no issues after scanning ${total_pkgs} package(s)." fi