open-telemetry / semantic-conventions

Defines standards for generating consistent, accessible telemetry across a variety of domains
Apache License 2.0
256 stars 165 forks source link

Adjust `process.command_args` and `process.command_line` to be opt-in #626

Open inssein opened 9 months ago

inssein commented 9 months ago

By default, auto instrumentation ships process.command_args, which is very dangerous as a lot of java services pass in secrets via command line arguments. Example:

java \
  -Dkeycloak.clientSecret="${KEYCLOAK_SECRET:-test}" \
  -jar app.jar

I have opened an issue against the java auto instrumentation repo (https://github.com/open-telemetry/opentelemetry-java-instrumentation/issues/10151), but I was pointed to https://opentelemetry.io/docs/specs/semconv/resource/process/ which indicates that the information I am asking to be made opt-in is marked "Conditionally Required" in the specification.

Does it make sense to give an out for languages where passing in secrets via command line arguments is common? Curious also if other languages have this problem and how they deal with it.

joaopgrassi commented 7 months ago

@open-telemetry/semconv-system-approvers FYI

mx-psi commented 7 months ago

I am in favor of making these opt-in

joaopgrassi commented 7 months ago

@inssein would you like to send a PR to address it?

inssein commented 7 months ago

@joaopgrassi happy to, but I just have a quick clarifying question.

For the container resource, comand_line and command_args are both opt_in. Are we good with doing that to the process resource, or just command_args?

trask commented 7 months ago

cc @breedx-splk since you had thoughts in https://github.com/open-telemetry/opentelemetry-java-instrumentation/issues/10151#issuecomment-1967818321

breedx-splk commented 7 months ago

cc @breedx-splk since you had thoughts in open-telemetry/opentelemetry-java-instrumentation#10151 (comment)

Thanks @trask. Yeah, I don't love the motivation here...tho I'm probably outnumbered.

We already jump through so many hoops to mask and hide information that users shouldn't be exposing in the first place, and I think that can come at the expense of troubleshooting utility. Like especially in the java world, where configuration items are often passed on the commandline via system properties. Not being able to see an agent's configuration (via commandline) after the fact is frustrating....and I use the commandline to help troubleshoot quite often.

In the other thread, there is also mention of size/length/waste, which I think is maybe a stronger reason for considering making these opt-in (off by default). I also pointed out that I think this is probably fine, since users and vendors can always choose to override it.

mx-psi commented 5 days ago

From 2024-10-03 system semantic conventions we all agree that we should make this opt-in, we just need to implement this