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

Actually e2e test java #3381

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

jaronoff97
Copy link
Contributor

Description:
Uses chainsaw to seed and then check for data in the e2e test for the java instrumentation only right now

Link to tracking Issue(s): #552

  • Resolves: #issue-number

Testing:

Documentation:

@jaronoff97 jaronoff97 added the Skip Changelog PRs that do not require a CHANGELOG.md entry label Oct 22, 2024
@jaronoff97 jaronoff97 requested a review from a team as a code owner October 22, 2024 20:32
@@ -9,11 +9,9 @@ spec:
- name: OTEL_EXPORTER_OTLP_ENDPOINT
value: http://localhost:4317
- name: OTEL_EXPORTER_OTLP_TIMEOUT
value: "20"
value: "20000"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is to account for the collector not being ready fast enough

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if it's not ready? Shouldn't this work correctly either way?

value: parentbased_traceidratio
- name: OTEL_TRACES_SAMPLER_ARG
value: "0.85"
value: always_on
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't want our test data to be sampled

@@ -41,3 +41,6 @@ spec:
traces:
receivers: [otlp]
exporters: [debug]
metrics:
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i was going to attempt the TLS test, but it seemed way too hard to do on a first-pass

@@ -20,3 +20,6 @@ spec:
traces:
receivers: [otlp]
exporters: [debug]
metrics:
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

java auto inst can send metrics too, why not accept them :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

@@ -473,7 +473,7 @@ KUSTOMIZE_VERSION ?= v5.0.3
CONTROLLER_TOOLS_VERSION ?= v0.16.1
GOLANGCI_LINT_VERSION ?= v1.57.2
KIND_VERSION ?= v0.20.0
CHAINSAW_VERSION ?= v0.2.8
CHAINSAW_VERSION ?= v0.2.11
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this version does not work on go 1.22. I would wait with updating before we upgrade the go version

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If that's a concern for us, we should install it by downloading the release binary instead. Our supported Go version shouldn't get in the way of tooling upgrades.

@@ -20,3 +20,6 @@ spec:
traces:
receivers: [otlp]
exporters: [debug]
metrics:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

#!/bin/bash
# set -ex
pod=$(kubectl get pods -n $NAMESPACE | awk '{print $1}' | tail -n 1)
kubectl get --raw /api/v1/namespaces/$NAMESPACE/pods/${pod}:8080/proxy/
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is new to me :)

@IshwarKanse does it work on OCP?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at this, I really wonder if we shouldn't use e2e_framework and/or terratest for these tests. The main problem is really getting data out of the cluster, as we don't have a remote we can easily query. I can think of a few solutions to do this in kind, but they won't work on OpenShift. It'd be best if these could simply be Go tests we could run on a host against any cluster.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be implemented natively using Chainsaw
and avoid using the script. https://github.com/kyverno/chainsaw/blob/main/testdata/e2e/examples/proxy/chainsaw-test.yaml

We were also discussing on implementing some kind of custom store in Chainsaw which gathers all the signals and would be useful for asserting. https://docs.google.com/document/d/1G4uDMrC3okF2q2qyNYTg2qj7yxNBjSTGgn8pBEXEeiw/edit#heading=h.8mm7ay5rgjs6

@eddycharly Do we have any decision on what custom store we can use for testing observability data. ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Skip Changelog PRs that do not require a CHANGELOG.md entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants