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

Fix agent startup with security manager #1208

Merged
merged 3 commits into from
Apr 16, 2023
Merged

Conversation

laurit
Copy link
Collaborator

@laurit laurit commented Apr 12, 2023

open-telemetry/opentelemetry-java-instrumentation#7983 adds an experimental option to improve compatibility with security manager to upstream agent. This pr adds a test for running our agent with enabled security manger and moves calls to methods that require privileges, such as System.setProperty and System.getenv, to a later point where agent code has the required privileges.

@laurit laurit requested review from a team as code owners April 12, 2023 09:16

@Override
public String name() {
return delegate.name();
Copy link
Contributor

Choose a reason for hiding this comment

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

There are now 2 LoggingCustomizers with the same name. Is it guaranteed that ours will always be the one that runs? Should we perhaps exclude the original one from the SPI file?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Currently all our services are listed before upstream services in the service files.
Excluding it seems like fine idea, but actually doing it will probably require an unhealthy amount of gradle magic. I think we would need build a transformer as described in https://imperceptiblethoughts.com/shadow/configuration/merging/#merging-service-descriptor-files WDYT is there a simpler way to do this? Should I attempt the transformer approach?

@breedx-splk
Copy link
Contributor

@laurit Can you please provide a brief description in this PR that explains the core problem and how this change improves it?

@laurit
Copy link
Collaborator Author

laurit commented Apr 12, 2023

@laurit Can you please provide a brief description in this PR that explains the core problem and how this change improves it?

@breedx-splk comment updated

@laurit laurit merged commit 6d8855d into signalfx:main Apr 16, 2023
@laurit laurit deleted the security-manager branch April 16, 2023 11:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants