-
Notifications
You must be signed in to change notification settings - Fork 614
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
docs: Getting started demo + sandbox #3584
docs: Getting started demo + sandbox #3584
Conversation
|
I'll be looking at this today and tomorrow. Thank you! |
@knylander-grafana just wondering if you had a chance to take a look? No rush :) |
Co-authored-by: Kim Nylander <[email protected]>
Co-authored-by: Kim Nylander <[email protected]>
Co-authored-by: Kim Nylander <[email protected]>
Hi @knylander-grafana sorry for the late turn around let me know if there are any other areas that need changed. Many thanks again for the review! |
You're welcome! I'll do another review today. |
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.
Thank you for updating the steps to the Explore Profiles app. The steps match what's in the Killercoda environment. Preview looks okay on the page and on Killercoda.
Most of my suggestions are cleanup from the Pyroscope UI to the Explore Profiles app.
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.
Not sure why this appeared twice.
This PR must be merged before a backport PR will be created. |
@Jayclifford345 I wanted to thank you for writing this tutorial. This is a great contribution. You signed the CLA, right? Weird that there is one message that says the CLA is needs to be signed and another one has been signed. Once we work through my comments, then we can get this merged. I'm so excited for this! I wonder if we could include it in the Explore Profiles docs too. What would you think about sharing the content between both locations? |
Co-authored-by: Kim Nylander <[email protected]>
Co-authored-by: Kim Nylander <[email protected]>
Yeah weird on the CLA I tried to sign again but it says I already signed. That should be all of your changes in thanks again for taking the time to go through it all and the corrections. Ready for your review once again. More than happy for it to go into the Explore Profiles section aswell. Let me know where you think its best place :) |
Okay! I'll review this today. :) let's have it in Pyroscope and look at whether we want to include in Explore Profiles too. I'll ask about sharing. |
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 great!
If this looks good to you, would you like me to merge? |
Hi @knylander-grafana, I sadly don't have permission to merge :(. If you wouldn't mind doing the honours! |
* Added getting started tutorial * added temp notice to sandbox * added play link * fixed typo * fixed typos * Update docs/sources/get-started/ride-share-tutorial.md --------- Co-authored-by: Kim Nylander <[email protected]> (cherry picked from commit f539340)
* Added getting started tutorial * added temp notice to sandbox * added play link * fixed typo * fixed typos * Update docs/sources/get-started/ride-share-tutorial.md --------- Co-authored-by: Kim Nylander <[email protected]> (cherry picked from commit f539340)
I have added a getting-started demo based on the ride-share app. This takes users through the basics of using Pyroscope to identify a bottleneck within the application code provided. This tutorial also provides a interactive sandbox for the user to try out this demo: https://killercoda.com/grafana-dev-testing/course/pyroscope/ride-share-tutorial
Unfortunately, there is an unusual bug within the Profile Explorer app (when using the sandbox) for which I am seeing help. For now, I have tagged this within the sandbox and once fixed will remove the notice. The rest of the sandbox which uses the Pyroscope UI works great! There is also a grafana play link so they can try this there
I belive for now the rest of the demo is good enough to get out there. Many thanks in advance for the review!