opentelemetry-ruby feat: consistent HTTP Request attributes with utility

This change introduces a utility that can be used by instrumentation to add consistent attributes for HTTP requests. Currently, HTTP attributes are set inconsistently across clients, which can be a bit of a headache if the services you're tracing have multiple HTTP client dependencies.

The utility also has an option to hide query parameters so that they don't show up in these attributes. Query parameters often hold sensitive information, which some (hopefully all 😅 ) teams may not want to log. I've made this on by default to align other recent changes to handling sensitive information.

I've applied this utility to the net/http, faraday, and http instrumentation, but I'd also want to apply this to all the other http clients in the repository.

May 24 '21 20:05 chrisholmes

@ericmustin, I was referring to the the comments made here rather than a specific doc: https://github.com/open-telemetry/opentelemetry-ruby/pull/737#discussion_r623876419

May 25 '21 18:05 chrisholmes

We discussed this on the weekly SIG call yesterday. These helpers don't belong in opentelemetry-common - that's already a bit of a grab bag of utilities that needs to be refactored. The specific concern is that the SDK depends on ...-common, as well as exporters required by the spec.

We would rather build an opentelemetry-instrumentation-helpers gem that holds helpers specific to instrumentation. The code in this PR belongs in that gem.

Additionally, we should think about common instrumentation options and how to ensure that related instrumentation uses the same option names, validations and defaults. Factoring those options out into modules that can be included in, e.g., HTTP client instrumentation, would be very useful.

May 26 '21 14:05 fbogsany

We discussed this on the weekly SIG call yesterday. These helpers don't belong in opentelemetry-common - that's already a bit of a grab bag of utilities that needs to be refactored. The specific concern is that the SDK depends on ...-common, as well as exporters required by the spec.

We would rather build an opentelemetry-instrumentation-helpers gem that holds helpers specific to instrumentation. The code in this PR belongs in that gem.

@fbogsany, this all sounds very sensible. I definitely would not have chosen to add to -common if there wasn't already some use. I'm happy to make the change, but would you prefer to see it in a separate PR that we can merge in before this one?

Additionally, we should think about common instrumentation options and how to ensure that related instrumentation uses the same option names, validations and defaults. Factoring those options out into modules that can be included in, e.g., HTTP client instrumentation, would be very useful.

Yes, there is definitely space for more reuse and abstractions here, but maybe too much for a single PR?

Would it help for me to join SIG to discuss the scope of this next week?

May 27 '21 10:05 chrisholmes

Would it help for me to join SIG to discuss the scope of this next week?

Sure, that's a great idea - thanks!

May 27 '21 13:05 fbogsany

Hello, and thank you for your contribution!

We recently split Ruby instrumentation out into the opentelemetry-ruby-contrib repo.

Portions of this PR are related to instrumentation gems, so we'll need you to re-open it against opentelemetry-ruby-contrib. Sorry for the inconvenience!

To do that, you can:

Create a fork of opentelemetry-ruby-contrib and copy the git url
In your opentelemetry-ruby repo, run git remote add tmp-contrib <your-fork-git-url>
git push tmp-contrib your-branch-name
Open a new PR in contrib (feel free to just copy/paste your original PR description there)
Close your open PR in this repo with a comment that links to your new PR in contrib
Delete your tmp-contrib remote from opentelemetry-ruby (git remote rm tmp-contrib)
git clone your opentelemetry-ruby-contrib fork, check out your branch, and make all changes in that repo from now on!

Sorry again for the inconvenience, and thank you for contributing!

Jun 17 '22 17:06 plantfansam

👋 Hi, @chrisholmes! Thank you for your PR!

As Sam mentioned, Ruby instrumentation now lives in the opentelemetry-ruby-contrib repo. We'd be happy to take another look at your work in that context.

Since most of this PR is instrumentation-related, I'm going to close it.

We appreciate your contribution and hope to work with you again soon.

Apr 01 '24 21:04 kaylareopelle