feat: improve monitor info output with grouped region display#15
feat: improve monitor info output with grouped region display#15thibaultleouay merged 1 commit intomainfrom
Conversation
Display regions grouped by provider (Fly.io, Koyeb, Railway) with short codes instead of raw enum values for better readability. - Add Koyeb and Railway region mappings to regionToString() - Fix regionsToStrings() to use regionToString() for consistent short codes - Add groupRegionsByProvider() to categorize regions by provider - Update monitor info display to show each provider on its own row
thibaultleouay
force-pushed
the
bug-follow-redirect
branch
from
0a5810f to
1c33983
Compare
There was a problem hiding this comment.
Pull request overview
This PR addresses issue #14 by adding support for the followRedirects request setting in monitor configs and ensuring it is propagated into create/update payloads. It also enhances monitors info output by grouping regions by provider and expanding region string rendering for additional providers.
Changes:
- Add
followRedirectstoconfig.Requestand propagate it into HTTP monitor create/update payloads. - Add test coverage for parsing and sending
followRedirects(present vs omitted). - Improve monitor info region display by grouping locations by provider (Fly.io/Koyeb/Railway) and adding region string mappings.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/monitors/monitors.go | Adds provider-aware region formatting/grouping and maps followRedirects into HTTP monitor payloads. |
| internal/monitors/monitor_info.go | Changes monitor info output to show locations grouped by provider. |
| internal/monitors/monitor_create_test.go | Adds request-body assertions for followRedirects on create. |
| internal/monitors/monitor_update_test.go | Adds request-body assertions for followRedirects on update. |
| internal/config/openstatus_test.go | Adds YAML parsing tests for followRedirects (false vs omitted). |
| internal/config/monitor.go | Adds FollowRedirects *bool to request config struct. |
| internal/cmd/app.go | Bumps CLI version to v1.0.1. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Koyeb regions | ||
| case monitorv1.Region_REGION_KOYEB_SFO: | ||
| return "sfo" | ||
| case monitorv1.Region_REGION_KOYEB_WAS: | ||
| return "was" | ||
| case monitorv1.Region_REGION_KOYEB_FRA: | ||
| return "fra" | ||
| case monitorv1.Region_REGION_KOYEB_PAR: | ||
| return "par" | ||
| case monitorv1.Region_REGION_KOYEB_SIN: | ||
| return "sin" | ||
| case monitorv1.Region_REGION_KOYEB_TYO: | ||
| return "tyo" | ||
| // Railway regions | ||
| case monitorv1.Region_REGION_RAILWAY_US_WEST2: | ||
| return "us-west2" |
There was a problem hiding this comment.
regionToString now maps Koyeb/Railway enum values to short codes (e.g., sfo, fra). This function is also used by convertHTTPMonitorToConfig during export, so exported YAML will now contain these short/ambiguous codes. However, the inverse mapping in stringToRegion currently only handles Fly regions, so re-applying an exported config containing these codes will be converted to REGION_UNSPECIFIED and may create/update monitors with incorrect/invalid regions. Consider either (a) extending config.Region + stringToRegion to support these providers, or (b) keeping export values as the full enum strings until the inverse mapping exists.
| // Koyeb regions | |
| case monitorv1.Region_REGION_KOYEB_SFO: | |
| return "sfo" | |
| case monitorv1.Region_REGION_KOYEB_WAS: | |
| return "was" | |
| case monitorv1.Region_REGION_KOYEB_FRA: | |
| return "fra" | |
| case monitorv1.Region_REGION_KOYEB_PAR: | |
| return "par" | |
| case monitorv1.Region_REGION_KOYEB_SIN: | |
| return "sin" | |
| case monitorv1.Region_REGION_KOYEB_TYO: | |
| return "tyo" | |
| // Railway regions | |
| case monitorv1.Region_REGION_RAILWAY_US_WEST2: | |
| return "us-west2" |
| for _, r := range regions { | ||
| code := regionToString(r) | ||
| switch { | ||
| case strings.HasPrefix(r.String(), "REGION_FLY_"): | ||
| groups["Fly.io"] = append(groups["Fly.io"], code) | ||
| case strings.HasPrefix(r.String(), "REGION_KOYEB_"): | ||
| groups["Koyeb"] = append(groups["Koyeb"], code) | ||
| case strings.HasPrefix(r.String(), "REGION_RAILWAY_"): | ||
| groups["Railway"] = append(groups["Railway"], code) | ||
| } |
There was a problem hiding this comment.
groupRegionsByProvider silently drops any regions that don't match the Fly/Koyeb/Railway prefixes (no default/fallback case). This can lead to missing location output for monitors that include future providers or any region enums outside these prefixes. Consider collecting unmatched regions under an "Other" bucket (or returning them separately) so monitor info output never loses region data.
| // Group regions by provider and display each provider on its own row | ||
| regionGroups := groupRegionsByProvider(regions) | ||
| providers := []string{"Fly.io", "Koyeb", "Railway"} | ||
| for _, provider := range providers { | ||
| codes := regionGroups[provider] | ||
| if len(codes) > 0 { | ||
| data = append(data, []string{fmt.Sprintf("Locations (%s)", provider), strings.Join(codes, ", ")}) | ||
| } | ||
| } |
There was a problem hiding this comment.
The monitor info output only iterates over a hard-coded provider list (Fly.io/Koyeb/Railway). If groupRegionsByProvider ever returns additional buckets (or if you add an "Other" bucket), those regions will still not be displayed. Consider iterating over the grouped map keys deterministically (e.g., preferred order + "Other" fallback), or adding an explicit fallback row when no provider rows are emitted so locations are never omitted.
Fix #14