envoy icon indicating copy to clipboard operation
envoy copied to clipboard

Implement setOperation for the opentelemetry provider

Open therealak12 opened this issue 9 months ago • 10 comments

Commit Message: implement setOperation for the opentelemetry provider

Additional Description: The detailed description is provided in issue#34063. In short, the MR implements a method for the OpenTelemetry tracing provider that was left empty and thus made the setOperation method a no-op.

Risk Level: Low

Testing: Unit testing and the sandbox.

Fixes issue#34063

therealak12 avatar May 09 '24 18:05 therealak12

Hi @therealak12, welcome and thank you for your contribution.

We will try to review your Pull Request as quickly as possible.

In the meantime, please take a look at the contribution guidelines if you have not done so already.

:cat:

Caused by: https://github.com/envoyproxy/envoy/pull/34062 was opened by therealak12.

see: more, trace.

The ci/do_ci.sh verify_examples check failed likely due to the following error. I'd appreciate any help on how to fix it.

#6 [wasm_compile_update stage-0 2/2] RUN --mount=type=cache,target=/var/cache/apt,sharing=locked     --mount=type=cache,target=/var/lib/apt/lists,sharing=locked     apt-get -qq install --no-install-recommends -y gosu     && groupadd -f envoygroup     && useradd -g envoygroup -m -d /home/envoybuild envoybuild
#6 0.224 E: Unable to locate package gosu
#6 ERROR: process "/bin/bash -ec apt-get -qq install --no-install-recommends -y gosu     && groupadd -f envoygroup     && useradd -g envoygroup -m -d /home/envoybuild envoybuild" did not complete successfully: exit code: 100
------
 > [wasm_compile_update stage-0 2/2] RUN --mount=type=cache,target=/var/cache/apt,sharing=locked     --mount=type=cache,target=/var/lib/apt/lists,sharing=locked     apt-get -qq install --no-install-recommends -y gosu     && groupadd -f envoygroup     && useradd -g envoygroup -m -d /home/envoybuild envoybuild:
0.224 E: Unable to locate package gosu
------
failed to solve: process "/bin/bash -ec apt-get -qq install --no-install-recommends -y gosu     && groupadd -f envoygroup     && useradd -g envoygroup -m -d /home/envoybuild envoybuild" did not complete successfully: exit code: 100

therealak12 avatar May 09 '24 20:05 therealak12

its actually a slightly different error (single-page-app) - the one posted above is known flakey so is caught as a warning - not sure exactly the cause

/retest

phlax avatar May 10 '24 08:05 phlax

Thanks for rerunning the Publish and verify job. Now The CodeQL/push... job is failing with java.io.IOException: No space left on device.

/retest

therealak12 avatar May 10 '24 14:05 therealak12

Haha. 😂 Inadvertently closed and reopened the pull request. I didn't know it would result in rerunning the jobs. By the way, the checks are all passed. 🥳

therealak12 avatar May 10 '24 16:05 therealak12

@therealak12 a sandbox test is helpful also, but probably this shoud have some code test if its critical to functionality

phlax avatar May 10 '24 16:05 phlax

@phlax It depends on how we define critical. Nonetheless, being compatible with Envoy docs is important enough to have tests for. So, thanks for reminding me. I'll add proper tests.

therealak12 avatar May 10 '24 17:05 therealak12

I'm going to mark the PR as draft till I add the tests.

therealak12 avatar May 10 '24 17:05 therealak12

/retest

therealak12 avatar May 11 '24 16:05 therealak12

@phlax I added a new test for the intended scenario. Marking the PR as ready for review.

therealak12 avatar May 11 '24 16:05 therealak12

Dear @htuch; Kindly please let me know if there's anything else I can do to move forward with the review process.

therealak12 avatar May 12 '24 19:05 therealak12