Skip to content

Conversation

@paracycle
Copy link
Contributor

The dalli instrumentation is defining a Utils module under the OpenTelemetry::Instrumentation. The Utils module should be an internal concern of the instrumentation library, and thus, the module should've been under the OpenTelemetry::Instrumentation::Dalli namespace.

Also, the file was already under the lib/opentelemetry/instrumentation/dalli folder, so the OpenTelemetry::Instrumentation::Dalli namespace is more appropriate.

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Apr 29, 2025

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: paracycle / name: Ufuk Kayserilioglu (97a6fc1)

@paracycle paracycle force-pushed the uk-fix-utils-classes branch from 4193881 to 2d23532 Compare April 29, 2025 19:33
@kaylareopelle kaylareopelle changed the title Do not pollute the OpenTelemetry::Instrumentation namespace fix: Do not pollute the OpenTelemetry::Instrumentation namespace Apr 29, 2025
@kaylareopelle
Copy link
Contributor

Hi @paracycle, thank you for opening this PR! Great catch. This will put Dalli in line with the other gems that have a Utils module. This looks good to me.

Would you mind merging main into your branch? (I don't have permission)

The `dalli` instrumentation is defining a `Utils` module under the `OpenTelemetry::Instrumentation`. The `Utils` module should be an internal concern of the instrumentation library, and thus, the module should've been under the `OpenTelemetry::Instrumentation::Dalli` namespace.
@paracycle paracycle force-pushed the uk-fix-utils-classes branch from 2d23532 to 97a6fc1 Compare April 29, 2025 21:32
@paracycle
Copy link
Contributor Author

Hi @paracycle, thank you for opening this PR! Great catch. This will put Dalli in line with the other gems that have a Utils module. This looks good to me.

Thanks for reviewing

Would you mind merging main into your branch? (I don't have permission)

Just rebased my commit on main

@kaylareopelle
Copy link
Contributor

The test failures seem unrelated to this PR, so I'm going to merge anyway. Thank you! I'll get a release started.

@kaylareopelle kaylareopelle merged commit de4da24 into open-telemetry:main Apr 30, 2025
69 of 105 checks passed
yiyuan-he pushed a commit to yiyuan-he/opentelemetry-ruby-contrib that referenced this pull request Apr 30, 2025
…pen-telemetry#1511)

Do not pollute the `OpenTelemetry::Instrumentation` namespace

The `dalli` instrumentation is defining a `Utils` module under the `OpenTelemetry::Instrumentation`. The `Utils` module should be an internal concern of the instrumentation library, and thus, the module should've been under the `OpenTelemetry::Instrumentation::Dalli` namespace.
@paracycle paracycle deleted the uk-fix-utils-classes branch May 1, 2025 20:10
yiyuan-he pushed a commit to yiyuan-he/opentelemetry-ruby-contrib that referenced this pull request May 14, 2025
…pen-telemetry#1511)

Do not pollute the `OpenTelemetry::Instrumentation` namespace

The `dalli` instrumentation is defining a `Utils` module under the `OpenTelemetry::Instrumentation`. The `Utils` module should be an internal concern of the instrumentation library, and thus, the module should've been under the `OpenTelemetry::Instrumentation::Dalli` namespace.
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