Conversation
Summary of ChangesHello @kamilkisiela, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the documentation for OpenTelemetry metrics in the Hive Router by introducing dedicated UI components for presenting metric and label information clearly. It provides users with a comprehensive guide on how to configure, export, and interpret the various metrics available, improving observability and troubleshooting capabilities for router operations. Highlights
Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
🚀 Snapshot Release (
|
| Package | Version | Info |
|---|---|---|
@graphql-hive/cli |
0.58.1-alpha-20260216151046-a7dc6de7cd0da593a8b7038d6ad153b0e21220a6 |
npm ↗︎ unpkg ↗︎ |
hive |
9.4.1-alpha-20260216151046-a7dc6de7cd0da593a8b7038d6ad153b0e21220a6 |
npm ↗︎ unpkg ↗︎ |
|
🐋 This PR was built and pushed to the following Docker images: Targets: Platforms: Image Tag: |
There was a problem hiding this comment.
Code Review
This pull request introduces comprehensive documentation for OpenTelemetry metrics, complete with new React components for a structured and interactive presentation. The implementation is solid, and the new documentation page is well-organized. I have a few suggestions to enhance semantic correctness, accessibility, and component reusability, primarily concerning the use of an <a> tag for a copy-link action and refactoring the MetricsSection component for better flexibility.
| <button | ||
| type="button" | ||
| onClick={() => { | ||
| void copyMetricLink(); | ||
| }} | ||
| className={`hive-focus inline-flex items-center justify-center rounded font-mono text-sm font-semibold leading-none transition-all duration-200 ${isCopied ? 'translate-y-0 text-gray-500 opacity-100 dark:text-slate-500' : 'translate-y-0 text-gray-500 opacity-0 hover:text-gray-700 focus:text-gray-700 group-focus-within:opacity-100 group-hover:opacity-100 dark:text-slate-500 dark:hover:text-slate-200 dark:focus:text-slate-200'}`} | ||
| aria-label={`Copy link to ${name}`} | ||
| title="Copy metric link" | ||
| > | ||
| {isCopied ? ( | ||
| <> | ||
| <span>✓</span> | ||
| <span className="ml-1 text-xs">copied</span> | ||
| </> | ||
| ) : ( | ||
| '#' | ||
| )} | ||
| </button> |
There was a problem hiding this comment.
For semantic correctness and better accessibility, consider changing this <button> to an <a> tag. The button's content is # and its fallback behavior involves navigation, which are characteristics of a link. Using an <a> tag aligns the element's semantics with its behavior and appearance, adhering to web standards and improving user experience, especially for users of assistive technologies.
<a
href={`#${metricId}`}
onClick={e => {
e.preventDefault();
void copyMetricLink();
}}
className={`hive-focus inline-flex items-center justify-center rounded font-mono text-sm font-semibold leading-none transition-all duration-200 ${isCopied ? 'translate-y-0 text-gray-500 opacity-100 dark:text-slate-500' : 'translate-y-0 text-gray-500 opacity-0 hover:text-gray-700 focus:text-gray-700 group-focus-within:opacity-100 group-hover:opacity-100 dark:text-slate-500 dark:hover:text-slate-200 dark:focus:text-slate-200'}`}
aria-label={`Copy link to ${name}`}
title="Copy metric link"
>
{isCopied ? (
<>
<span>✓</span>
<span className="ml-1 text-xs">copied</span>
</>
) : (
'#'
)}
</a>
References
- For navigation, use an
<a>tag (e.g., via a router'sLinkcomponent) instead of a<button>element to ensure semantic correctness and accessibility.
| title?: string; | ||
| description?: string; |
| <div className="space-y-4"> | ||
| <h4 className="mt-8 text-xl font-semibold tracking-tight text-slate-900 dark:text-slate-100"> | ||
| Metrics | ||
| </h4> | ||
| <div className="grid gap-4"> | ||
| {metrics.map(metric => ( | ||
| <MetricCard key={metric.name} {...metric} /> | ||
| ))} | ||
| </div> | ||
| </div> |
There was a problem hiding this comment.
The component includes a hardcoded <h4>Metrics</h4> heading. This can lead to incorrect heading hierarchies when used within metrics.mdx (e.g., an h4 as a direct child of another h4) and reduces the component's flexibility. It's better to remove this heading and let the consumer of the component (the MDX file) control the document structure and headings.
<div className="grid gap-4">
{metrics.map(metric => (
<MetricCard key={metric.name} {...metric} />
))}
</div>
💻 Website PreviewThe latest changes are available as preview in: https://pr-7696.hive-landing-page.pages.dev |
graphql-hive/router#770