-
Notifications
You must be signed in to change notification settings - Fork 377
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
Rename library to datadog #3490
Conversation
Notes about development There are quite a few changes here, and I think its worth segmenting them into what's a minimum, and what's additional. Changes
Open questions
|
32f23fa
to
bcee292
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks fine at first glance for non-profiling changes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 This seems reasonable to me!
My two main concerns are:
a. Smooth ddprofrb
migration path
b. Changes to profiling stuff makes it hard to move stuff between master and 2.0 branch
And actually I think we can solve both a. and b. by moving most of the profiling changes to the master branch (e.g. as a separate PR).
Starting with a., we could introduce a bin/ddprofrb
, side-by-side with the old bin/ddtracerb
. The old ddtracerb
would print a deprecation warning when getting used, and so customers on the latest 1.x would be able to move to the new name even before moving to 2.0, and would not be "surprised" when it's gone in 2.0. Then in the 2.0 branch we can remove it.
In the case of b., almost all of the changes to profiling (name of the extensions, build_ddtrace_profiling_native_extension
script, etc) are internal things that are not part of any public API, so by making those changes in the master branch we'd simplify back/forward porting AND also make this PR a lot simpler as well.
Thoughts?
|
||
## Special thanks | ||
|
||
* [Mike Fiedler](https://github.com/miketheman) for working on a number of Datadog Ruby projects, as well as graciously | ||
gifting control of the `datadog` gem |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mike kindly asked to be given credit for helping out in getting us the datadog gem back, so I would suggest keeping this around :)
COPY . /vendor/dd-trace-rb | ||
|
||
# Install dependencies | ||
# Setup specific version of ddtrace, if specified. | ||
ENV DD_DEMO_ENV_GEM_LOCAL_DDTRACE /vendor/dd-trace-rb | ||
# Setup specific version of datadog, if specified. | ||
ENV DD_DEMO_ENV_GEM_LOCAL_DATADOG /vendor/dd-trace-rb |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess these dd-trace-rb
paths should be updated too
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Technically these reflect the repo name; we can change these now (say to datadog-rb
) if we want, but it would be out of step with the current repo name. I recommend holding off and doing repo renaming changes separately.
@ivoanjo I'm onboard with your plan. I think we were hoping to do another 1.x release to add deprecation warnings anyways, so introducing the new |
4638756
to
b14a157
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## 2.0 #3490 +/- ##
=======================================
Coverage 98.07% 98.07%
=======================================
Files 1237 1237
Lines 72254 72254
Branches 3408 3408
=======================================
+ Hits 70863 70864 +1
+ Misses 1391 1390 -1 ☔ View full report in Codecov by Sentry. |
8ee5357
to
bad2498
Compare
Rebased this branch on the latest 2.0 and added one more name change of Handing off to @TonyCTHsu and @marcotc for any relevant system tests, benchmarks, etc... |
8bea400
to
3541f74
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Documentation updates look good.
19b437d
to
cd9d31c
Compare
Rename the library from
ddtrace
todatadog
to reflect the inclusive nature of many Datadog tools (tracing, profiling, ASM, etc).2.0 Upgrade Guide notes
Users should use
datadog
instead ofddtrace
in their requires, where applicable e.g.:gem 'ddtrace'
-->gem 'datadog'
require 'ddtrace'
-->require 'datadog'
DDTrace::VERSION
-->Datadog::VERSION
It otherwise preserves all the same functionality & default behavior present in
ddtrace
.What does this PR do?
A 1-1 name change of the
ddtrace
gem todatadog
, updating relevant file names, constants, comments, and other references to the Ruby gem.Motivation:
ddtrace
as a name has not been reflective of the many products we include within the package, and the name was in conflict with the semantic conventions of Ruby (where gem name & file paths should mirror constants.)Additional Notes:
TBD
How to test the change?
TBD