-
Notifications
You must be signed in to change notification settings - Fork 1.2k
[APM Go] clarify profiling pages for Go Tracer v2 #28266
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
Conversation
Preview links (active after the
|
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.
Hm, I don't think this PR clarifies enough.
What we need is a link to a page that explains:
- What is v2?
- Where does it sit in the product lifecycle? (preview?)
- Should I use it for new projects?
- Should I migrate? (link to migration docs)
- What's are the major differences between v1 and v2, by product? (for profiler this means explicitly stating that there are no major changes)
IMO #25897 created a very confusing situation for customers, and it's not limited to profiling. So my comment applies to all of our mentions for v2.
So I would strongly encourage considering making a larger docs update for this.
Anyway, I made some inline comments that should help getting the profiling docs into a better shape in the meantime.
content/en/profiler/enabling/go.md
Outdated
| go get gopkg.in/DataDog/dd-trace-go.v1/profiler # v1 | ||
| # go get github.com/DataDog/dd-trace-go/v2/profiler # v2 | ||
| go get gopkg.in/DataDog/dd-trace-go.v1/profiler # 1.x | ||
| # go get github.com/DataDog/dd-trace-go/v2/profiler # 2.x |
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.
Why 1.x and 2.x? The old v1 or v2 comments were fine IMO.
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 wanted to keep the comments consistent between here and the code on lines 51/52 so that the new instructions at the top would make more sense. I'm open to changing both to either v1/v2 or 1.x/2.x; what do you think?
|
@felixge Good points; thank you for the review! Agree that we could clarify some points throughout the docs. Let's fix the profiler page for now, and I'll open another PR to start addressing some other issues. |
|
@felixge - I made another iteration in 09f19bf. Let me know what you think. I thought we could further simplify this for users by focusing primarily on v2, especially since this page is for new users installing the profiler. Per @darccio , new users should use v2 moving forward (after GA). Agree on your note about more context needed. I'll work to flesh out the migration page a bit more in Hannah's separate PR. But I think for the sake of simplicity, most users who come to this page shouldn't have to worry about v1 at all. They should be directed to the newest version that will continue receiving updates and features. |
|
@felixge - I also enhanced our migration guide to be a better source of information for our users in Hannah's separate PR. Preview (Note: Still needs review). |
|
Closing in favor of using #28292 |
What does this PR do? What is the motivation?
Clarifies profiling docs to be less confusing regarding the dd-trace-go v2 release.
Merge instructions
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