Skip to content

Conversation

@revmag
Copy link
Collaborator

@revmag revmag commented Oct 30, 2025

Adding fly.io docs integration guide along with dashboards.

@vercel
Copy link

vercel bot commented Oct 30, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
signoz-web Ready Ready Preview Comment Dec 1, 2025 2:48pm

@makeavish
Copy link
Member

@gemini-cli /review

@github-actions
Copy link
Contributor

github-actions bot commented Nov 3, 2025

🤖 Hi @makeavish, I've received your request, and I'm working on it now! You can track my progress in the logs for more details.

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📋 Review Summary

This Pull Request introduces new documentation for Fly.io metrics integration, including setup instructions and dashboard visualization. The changes are well-structured and follow most of the documentation guidelines.

🔍 General Feedback

  • The new documentation for Fly.io metrics is comprehensive and provides clear steps for integration.
  • The use of code blocks with language and filename annotations is excellent for clarity.
  • Ensure all external links adhere to the CONTRIBUTING.md guidelines for opening in new tabs with security attributes.
  • Consider the use of tabs for Cloud vs Self-Host sections, as per CONTRIBUTING.md recommendations.

@revmag
Copy link
Collaborator Author

revmag commented Nov 3, 2025

@makeavish please review the fly.io dashboard docs first, the link for dashboard is temperory in this doc, once that is approved, will update

@makeavish
Copy link
Member

@claude /review

@claude
Copy link
Contributor

claude bot commented Nov 26, 2025

Claude finished @makeavish's task —— View job


Review Complete

I've reviewed the Fly.io metrics documentation PR against the CONTRIBUTING.md guidelines. Here's my assessment:

Technical Accuracy ✅

The documentation is technically accurate. I verified:

  • The /federate endpoint path (api.fly.io/prometheus/<org-slug>/federate) is correct (Source)
  • The FlyV1 token authentication format is accurate (Source)
  • The note about metric types defaulting to Gauge is correct - Fly.io's federation endpoint doesn't expose metric types (Source)

Issues Found

1. Missing doc_type in Frontmatter ⚠️

Line 7: The frontmatter is missing the required doc_type field. According to CONTRIBUTING.md, all docs must specify one of: tutorial, howto, reference, or explanation.

Recommendation: Add doc_type: howto since this is a task-focused guide for sending Fly.io metrics to SigNoz.

---
date: 2025-11-25
title: Fly.io Metrics
id: fly-metrics
description: Monitor Fly.io application metrics in SigNoz
tags: [SigNoz Cloud, Self-Host]
doc_type: howto
---

2. External Links Missing Security Attributes ⚠️

Lines 191, 202: External links to GitHub should open in a new tab with security attributes as per CONTRIBUTING.md guidelines.

Current:

[here](https://github.com/SigNoz/dashboards/blob/main/fly/fly-prometheus-v1.json)

Should be:

<a href="https://github.com/SigNoz/dashboards/blob/main/fly/fly-prometheus-v1.json" target="_blank" rel="noopener noreferrer nofollow">Fly.io dashboard JSON</a>

Avoid using "here" as link text - use descriptive anchor text instead.

3. Missing ## Troubleshooting Section

The guidelines require a Troubleshooting section with common issues and fixes. The doc briefly mentions "If you see 'failed to scrape' or auth errors, re-check the token step" (line 166) but should have a dedicated section covering:

  • Authentication/token issues (invalid token, wrong format)
  • Network connectivity problems
  • Missing metrics scenarios
  • Common configuration errors

4. Missing ## Next Steps Section

Consider adding a "Next Steps" section that links to:

  • How to create alerts based on Fly.io metrics
  • Query builder documentation for custom queries
  • Related monitoring guides

This follows the CONTRIBUTING.md guideline: "Close the loop with next steps: Link to relevant dashboards or dashboard templates, example alerts, service and trace views, and deeper user guides."

What's Good ✅

  • Clear step-by-step instructions
  • Code snippets are well-annotated with language and filename
  • Proper use of tabs for Cloud vs Self-Host
  • Validation steps included
  • Sidebar entry correctly added
  • Dashboard visualization included
  • Technical accuracy verified

Label Recommendation

add-to-onboarding label needed: This PR adds a NEW documentation file explaining how to send metrics data to SigNoz (Fly.io metrics integration). According to the repository guidelines, this qualifies for the add-to-onboarding label. Please add this label to trigger the onboarding workflow.


Summary

The documentation is well-structured and technically accurate. To meet CONTRIBUTING.md standards, please address:

Required:

  1. Add doc_type: howto to frontmatter
  2. Fix external link security attributes (use <a> tags with proper attributes)

Recommended:
3. Add dedicated ## Troubleshooting section
4. Add ## Next Steps section with follow-through links
5. Improve link text (avoid "here")
6. Add the add-to-onboarding label


@makeavish
Copy link
Member

makeavish commented Nov 26, 2025

@revmag Address claude comments

@github-actions
Copy link
Contributor

📚 Onboarding issue created… see #2267

@makeavish
Copy link
Member

@claude /review

@claude
Copy link
Contributor

claude bot commented Dec 1, 2025

Claude finished @makeavish's task —— View job


Review Complete

I've reviewed the Fly.io metrics documentation PR. The author has addressed most of the previous review comments. Here's my assessment:


What's Been Fixed ✅

The following issues from the previous review have been addressed:

  1. doc_type added - The frontmatter now includes doc_type: howto (line 7)
  2. External links fixed - GitHub dashboard links now use proper <a> tags with security attributes (lines 379, 390)
  3. Troubleshooting section added - A dedicated collapsible ## Troubleshooting section is present (lines 354-373)
  4. Next Steps section added - Includes links to alerts and dashboards (lines 375-394)

Remaining Issues

1. Missing className: 'new-doc' in Sidebar Entry ⚠️

File: constants/docsSideNav.ts:1586-1590

The sidebar entry is missing the className: 'new-doc' property. According to CONTRIBUTING.md, new documentation entries should include this to highlight them in the sidebar.

Current:

{
  type: 'doc',
  route: '/docs/metrics-management/fly-metrics',
  label: 'Fly.io metrics',
},

Should be:

{
  type: 'doc',
  route: '/docs/metrics-management/fly-metrics',
  label: 'Fly.io metrics',
  className: 'new-doc',
},

2. Missing VM Explanation Callout

File: data/docs/metrics-management/fly-metrics.mdx:154

According to CONTRIBUTING.md guidelines for Send Data docs covering multiple deployment types, the VM tab should include an explanation callout at the start:

<KeyPointCallout defaultCollapsed={true}>
A VM is a virtual computer that runs on physical hardware. This includes:
- **Cloud VMs**: AWS EC2, Google Compute Engine, Azure VMs, DigitalOcean Droplets
- **On-premise VMs**: VMware, VirtualBox, Hyper-V, KVM
- **Bare metal servers**: Physical servers running Linux/Unix directly

Use this section if you're deploying your application directly on a server or VM without containerization.
</KeyPointCallout>

This should be added right after the VM tab opens (after line 154).

3. Inconsistent Heading Levels in Tabs

File: data/docs/metrics-management/fly-metrics.mdx:139, 212

The "Step 5: Run the Collector" heading appears as ### Step 5 inside the Docker tab (line 139), but the VM tab has the same step at line 212 without being inside a proper step structure. The Kubernetes tab doesn't have a "Run" step at all. This creates inconsistency across deployment types.

Recommendation: Ensure all deployment tabs follow the same step structure for consistency.


Technical Accuracy ✅

I verified the technical details:

  • Federation endpoint path (api.fly.io/prometheus/<org-slug>/federate) is correct (Source)
  • FlyV1 token authorization format is accurate (Source)
  • Metric type handling note about metrics defaulting to Gauge is correct - Fly.io's federation endpoint doesn't expose metric types

What's Good ✅

  • Clear step-by-step instructions with proper code annotations
  • Proper use of tabs for different deployment types (Docker, VM, Kubernetes)
  • Self-hosted callout included (lines 12-14)
  • Validation steps included with manual curl test
  • Troubleshooting section with concrete fixes
  • Dashboard visualization and import instructions
  • Technical accuracy verified

Summary

Required Changes:

  1. Add className: 'new-doc' to the sidebar entry in constants/docsSideNav.ts
  2. Add VM explanation callout at the start of the VM tab

Optional Improvements:
3. Standardize step structure across all deployment tabs for consistency

The documentation is well-structured and technically accurate. The previous review comments have been properly addressed. Once the sidebar className is added and the VM callout is included, this will fully comply with CONTRIBUTING.md standards.

Note: The add-to-onboarding label requirement was already fulfilled (issue #2267 was created on 2025-11-26).


Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants