-
Notifications
You must be signed in to change notification settings - Fork 369
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
1.0 upgrade guide restructure #1914
Conversation
0903139
to
5e68612
Compare
Codecov Report
@@ Coverage Diff @@
## master #1914 +/- ##
==========================================
- Coverage 97.51% 97.51% -0.01%
==========================================
Files 1001 1000 -1
Lines 48795 48787 -8
==========================================
- Hits 47581 47573 -8
Misses 1214 1214
Continue to review full report at Codecov.
|
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.
Thanks for working on this! I've did an extensive read-through of it again, and left a lot of suggestions. I think all the time we're putting into this guide is worth it, as it will mean less work for customers and a silky-smooth upgrade experience.
384ed30
to
fdc0594
Compare
fdc0594
to
1daa257
Compare
1daa257
to
721ab5d
Compare
**How to upgrade basic usage** | ||
|
||
Here's a list of the most common changes you may encounter: | ||
For users with a basic implementation (configuration file + out-of-the-box instrumentation), only minor changes to your configuration file are required: most applications take just minutes to update. Check out the following sections for a step-by-step guide. |
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'm still quite unconvinced with the use of the word basic. Since you didn't like my suggestion of standard, I've gotten my thesaurus out and here's a few more suggestions: base, common, simple, typical, regular, bourgeois, philistine.
All of them I think don't carry the weird connotations that sometimes get attached to basic 😄
|
||
### Removed `Datadog.tracer` | ||
<h1 id="1.0-basic-upgrade">Upgrading basic usage</h1> |
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.
It's a bit weird that while this document (UpgradeGuide.md
) seems like it's making space for future upgrades to be included as well, most sections are using h1's, aka putting stuff on the top-level structure.
Should we make this document just the 1.x upgrade guide? In that case, it makes sense to keep the h1s. Otherwise, should everything below "From 0.x to 1.0" become an h2?
Also check out the functions defined within `Datadog::Tracing` in our [public API](https://www.rubydoc.info/gems/ddtrace/) for more details on their usage. | ||
|
||
<h3 id="1.0-trace-api-removed-tracer">Removed `Datadog.tracer`</h3> | ||
|
||
Many of the functions accessed directly through `Datadog.tracer` have been moved to `Datadog::Tracing` instead. | ||
|
||
<h3 id="1.0-trace-api-removed-context">Removed access to `Datadog::Context`</h3> | ||
|
||
Manual tracing is now done through the public API. | ||
Direct usage of `Datadog::Context` has been removed. Previously, it was used to modify or access active trace state. Most use cases have been replaced by our [public trace API](https://www.rubydoc.info/gems/ddtrace/). | ||
|
||
Whereas in 0.x, the block would yield a `Datadog::Span` as `span`, in 1.0, the block yields a `Datadog::SpanOperation` as `span` and `Datadog::TraceOperation` as `trace`. | ||
<h3 id="1.0-trace-api-manual-tracing">Manual tracing & trace model</h3> | ||
|
||
Manual tracing is now done through the [public API](https://www.rubydoc.info/gems/ddtrace/). |
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.
The rubydoc.info site hasn't yet been updated with 1.0.0.beta1 (probably only indexes non-beta versions?) but I think it would be nice to update this document after release with deeper links to specific classes/parts of the API. (So not for this PR, just a suggestion for a later improvement)
|
||
Direct usage of `Datadog::Context` has been removed. Previously, it was used to modify or access active trace state. Most use cases have been replaced by our public trace API. | ||
|
||
<h3 id="1.0-trace-api-manual-tracing">Manual tracing & trace model</h3> | ||
|
||
Manual tracing is now done through the public API. | ||
|
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.
Hmm I've reviewed the change and I still think it's not as clear is it could be. It's now "the block would provide" and "the block provides", where it's actually the reverse: the block gets provided/called -- it's on the receiving end, not on the supplying end.
|
||
<h3 id="1.0-trace-api-removed-tracer">Removed `Datadog.tracer`</h3> | ||
|
||
Many of the functions accessed directly through `Datadog.tracer` have been moved to `Datadog::Tracing` instead. |
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.
Would it make sense to point to some other part of the document (appendix?) to easily jump to a definitive reference to what has been moved? Or maybe to the class on rubydoc.info?
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.
The guide has the Trace API
which has a table that outlines most API usage and the equivalent. I opted to put this inline rather than as a reference in the appendix.
My thinking was putting critical info (that isn't overwhelming) in the main body minimizes jumps and keeps flow/orientation smooth. Any tabular information that is both 1) verbose/detail oriented, 2) not-always-applicable can be demoted to an appendix section to reduce length and improve readability.
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.
Overall a good improvement, I feel it does make things a bot more approachable and "drillable" so to speak.
721ab5d
to
c49296f
Compare
To try to improve comprehension and ease of use, I've restructured the 1.0 upgrade guide a bit here.
Basic usage
andAdvanced usage
path.Trace API
section with table of common mappings.Namespacing
section, added table.1.0
(for future-proofing)