Move agent config and its dependencies into agent classloader

hypertrace / javaagent

Hypertrace OpenTelemetry Java agent with payload/body and headers data capture.

Apache License 2.0

30 stars 15 forks source link

Move agent config and its dependencies into agent classloader #322

Closed pavolloffay closed 3 years ago

pavolloffay commented 3 years ago

Signed-off-by: Pavol Loffay p.loffay@gmail.com

Description

Resolves #313

Notable changes:

config and its dependencies are moved to agent classloarder. The classes in the agent classloader are totally isolated from the application.
shading for proto and jackson is removed - not needed anymore
added a new config interface that is backed by old config class. The new interface is located in javaagent-core that lives in the bootstrap classloader. The binding between the interface and proto-based config is done at javaagent bootstrap via component installer SPI.

The whole idea is that javaagent-core is located in the bootstrap classloader hence it should not bring any other 3rd party dependencies. The otel-extensions is located in the agent classloader and can load and use anything bc it is fully isolated from the application.

pavolloffay commented 3 years ago

@ryandens it seems that snyk still fails on the PRs from forks.

pavolloffay commented 3 years ago

The CI passes not. Snyk is misconfigured and fails on PRs from forks on

"error-message": "snyk requires an authenticated account. Please run snyk auth and try again.",

pavolloffay commented 3 years ago

The body capture works, tested on https://github.com/grpc/grpc-java/tree/master/examples

Screenshot of Jaeger UI

pavolloffay commented 3 years ago

do not merge yet it needs more love - e.g. remove the filter paths from the instrumentation config. That should not be available to instrumentations.

pavolloffay commented 3 years ago

@shashank11p PR updated, could you please re-review?

ryandens commented 3 years ago

@ryandens it seems that snyk still fails on the PRs from forks.

Hey Pavol! Sorry about that would you mind trying out a fix to help this PR along? I think we need to

Swap out --org=hypertrace with --org=$GITHUB_ACTOR
Depending on whether you've done this before, you'll need to login and authorize your github account.

pavolloffay commented 3 years ago

I am not sure if that will work, most likely the secret is not present in PRs from forks https://github.com/pavolloffay/javaagent/blob/52f0a2d388ce5951e4627f6f18c0fa0f9e4f12a1/.github/workflows/build.yaml#L82.

This repository has the same snyk setting as other HT repos e.g. https://github.com/hypertrace/hypertrace-service/blob/main/.github/workflows/pr-build.yml#L79. I will leave this as it is, somebody from the team should fix it so that PRs from the community are not turning red.

EDIT: snyk passed on https://github.com/hypertrace/hypertrace-service/pull/97. I will look at it in a different PR