-
Notifications
You must be signed in to change notification settings - Fork 1.2k
[APM Go] fix and clarify dd-trace-go v2 docs for release #28292
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
base: master
Are you sure you want to change the base?
Conversation
Most important to review here is whether statements left under DSM and OTel files are accurate; has v2 made significant changes to their APIs? I don't think it has, but I'd like to confirm this. |
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 left some comments. But since this is still draft, I'm not sure if I should try to do a complete review at this point? LMK
content/en/tracing/trace_collection/custom_instrumentation/go/migration.md
Outdated
Show resolved
Hide resolved
content/en/tracing/trace_collection/custom_instrumentation/go/migration.md
Show resolved
Hide resolved
content/en/tracing/trace_collection/custom_instrumentation/go/migration.md
Outdated
Show resolved
Hide resolved
content/en/tracing/trace_collection/custom_instrumentation/go/migration.md
Outdated
Show resolved
Hide resolved
|
||
### Package structure changes | ||
|
||
The package organization has changed in v2. Many functions previously in `ddtrace/tracer` have been moved to the `ddtrace` package. While the `v2check` migration tool handles these changes automatically, you may need to manually update some import paths. |
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.
Did we have a good reason for making this change? If yes, maybe we can communicate it here?
Edit: On second thought, did we even really make this change 🤔? Looking at the v2-dev branch, it doesn't seem to be the case?
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 like a typo! I think it should be that functions previously in ddtrace
are now in ddtrace/tracer
. Thanks!
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 also looks like the examples were incorrect. @darccio I updated them to be more representative, but let me know if there's a more concise/better example of the ddtrace
--> ddtrace/tracer
change?
content/en/tracing/trace_collection/custom_instrumentation/go/migration.md
Show resolved
Hide resolved
@felixge Thank you for the eyes! Might as well make this PR open to review. 🏁 |
What does this PR do? What is the motivation?
To minimize confusion with the differences between v1 and v2, we clarify when there are not significant changes to the specific product and link customers to the migration guide whenever possible.
Also see profiling changes here.
Swap v2 code statements to come first ORremove v1 import statements entirely.Merge instructions
Merge whenever possible, preferably before or around the dd-trace-go v2 GA release.
Merge readiness:
Merge queue is enabled in this repo. Your branch name MUST follow the
<slack_username>/<branch_name>
convention, or your pull request will not pass in CI. If your branch doesn't follow this format, rename it or create a new branch and PR.To have your PR automatically merged after it receives the required reviews, add the following PR comment:
Additional notes