docs(hono): Document dedicated @sentry/hono SDK#17557
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
inventarSarah
left a comment
There was a problem hiding this comment.
Hey @s1gr1d 👋
left some questions and suggestions -- thank you!
| - **Cloudflare Workers**: Use `@sentry/cloudflare` – see our [Hono on Cloudflare guide](/platforms/javascript/guides/cloudflare/frameworks/hono/) | ||
| - **Deno**: Use `@sentry/deno` – see our [Deno guide](/platforms/javascript/guides/deno/) (Beta) | ||
| - **Bun**: Use `@sentry/bun` – see our [Bun guide](/platforms/javascript/guides/bun/) | ||
| The community middleware `@hono/sentry` has been deprecated in favor of `@sentry/hono`, which provides better performance and more features. If you're currently using `@hono/sentry`, see the [migration section](#migrating-from-honosentry) below. |
There was a problem hiding this comment.
You could create an Expandable section "Are you using @hono/sentry?", add this content inside it, and then also the migration section from the end of the guide.
This would still only require minimal space, and it removes the migration part at the end, which currently looks a bit lost.
What do you think?
| options={["error-monitoring", "performance", "logs"]} | ||
| /> | ||
|
|
||
| <Include name="quick-start-features-expandable" /> |
There was a problem hiding this comment.
This still lists "Profiling" -> you can update this in the include file
There was a problem hiding this comment.
I think I'll just add profiling as people can still use it when they are on the Node runtime.
|
|
||
| ## Configure | ||
|
|
||
| <Expandable title="Cloudflare Workers: Enable Node.js Compatibility"> |
There was a problem hiding this comment.
What is our plan regarding the Hono on Cloudflare guide?
Currently, the installation of the hono SDK is missing here (otherwise it looks alright?). If we want to keep the CF guide, we could remove the Cloudflare instructions from this guide and link to Cloudflare.
There was a problem hiding this comment.
True, it makes sense to include this to the list of frameworks there. As this is using the cloudflare sdk under the hood, it makes sense to already be in the cloudflare docs.
However, we could also link from cloudflare to the hono docs? Then we would have the hono docs in one place and just need different code snippets. What do you think @inventarSarah ?
There was a problem hiding this comment.
Yes, that sounds okay to me.
It's definitely easier for us to maintain.
So we need to:
- replace the link to Hono on the CF quick start guide
- remove the content from "hono on CF" and link to the Hono quick start guide instead (I don't think we can link to the Hono guide directly from the CF navigation, right?)
That should be it, I think 🤔
inventarSarah
left a comment
There was a problem hiding this comment.
Thanks for the updates!
We only need to update Hono on CF :)
| <SplitSection> | ||
| <SplitSectionText> | ||
|
|
||
| Profiling is available for the Node.js runtime. Install the `@sentry/profiling-node` package: |
There was a problem hiding this comment.
| Profiling is available for the Node.js runtime. Install the `@sentry/profiling-node` package: | |
| Profiling is only available for the Node.js runtime. Install the `@sentry/profiling-node` package: |
adding "only" here again just to make sure users don't miss this
|
|
||
| ## Configure | ||
|
|
||
| <Expandable title="Cloudflare Workers: Enable Node.js Compatibility"> |
There was a problem hiding this comment.
Yes, that sounds okay to me.
It's definitely easier for us to maintain.
So we need to:
- replace the link to Hono on the CF quick start guide
- remove the content from "hono on CF" and link to the Hono quick start guide instead (I don't think we can link to the Hono guide directly from the CF navigation, right?)
That should be it, I think 🤔
chargome
left a comment
There was a problem hiding this comment.
Just found a small bug, other than that lgtm!
| ```bash | ||
| node --import ./instrument.mjs app.js | ||
| ``` | ||
|
|
||
| ```bash | ||
| NODE_OPTIONS="--import ./instrument.mjs" | ||
| ``` |
There was a problem hiding this comment.
This does not render properly with the same title, you might need to give it a filename or rename the tab title it seems
DESCRIBE YOUR PR
The Hono docs previously documented the setup using other SDKs, but not
@sentry/hono. This PR changes the quick setup page to include the new setup.Closes getsentry/sentry-javascript#20402
IS YOUR CHANGE URGENT?
Help us prioritize incoming PRs by letting us know when the change needs to go live.
SLA
Thanks in advance for your help!
PRE-MERGE CHECKLIST
Make sure you've checked the following before merging your changes: