search

open-telemetry / opentelemetry-collector-releases

OpenTelemetry Collector Official Releases
https://opentelemetry.io
Apache License 2.0
252 stars 161 forks source link

Enabling debug symbols #58

Open aateeqi opened 2 years ago

aateeqi commented 2 years ago

We are contemplating whether to enable debug symbols (disabled by -s -w LDFLAGS on build) on our downstream repo aws-otel-collector. We want to be in line with what is being done upstream but it seems that there's an inconsistency. In this opentelemetry-collector-releases repo it appears that they are disabled but in opentelemetry-collector-contrib it appears to be enabled.

Is there any reason for this inconsistence/any guidance on what we should lean towards regarding enabling/disabling debug symbols?

jpkrohling commented 2 years ago

Are there performance penalties in having debug symbols enabled?

jpkrohling commented 2 years ago

@kaero, @aateeqi, would you be open to opening a PR for this?

kaero commented 2 years ago

@jpkrohling

@kaero, @aateeqi, would you be open to opening a PR for this?

Yes, I would like to implement the feature in the builder in a backward-compatible way as it suggested in https://github.com/open-telemetry/opentelemetry-collector-releases/issues/63#issue-1113893865 , but I have no idea who and how use releases issued from this repository. We make private builds of the collector using the builder.

I could update builder manifests here too if it would be helpful for users of those releases.

kaero commented 2 years ago

Are there performance penalties in having debug symbols enabled?

Generally speaking, no. Debug symbols are used by debugging facilities in cases they required to, for example when a user attach or run process under a debugger or the Linux perf tool. The only effect everyone who don't care about debugging should notice is the growth of the binary size.

jpkrohling commented 2 years ago

I believe the only place you'd need to change is this: https://github.com/open-telemetry/opentelemetry-collector-releases/blob/337b16662f716559b0b1dbae5707942f59ec8c18/goreleaser/configure.go#L76

kaero commented 2 years ago

I believe the only place you'd need to change is this:

I don't see, how it would help binaries built using the OpenTelemetry builder command line tool.

jpkrohling commented 2 years ago

Sorry, I missed that part!

We make private builds of the collector using the builder.

This would be the place to remove it: https://github.com/open-telemetry/opentelemetry-collector/blob/88e9c86f2d32c224a488a5a4935cdafcafc5b943/cmd/builder/internal/builder/main.go#L89

I think we can externalize it in a config property.