Skip to content

fix: restore beforeExit handler for version commands and add comprehensive telemetry debug logging#3657

Merged
eablack merged 2 commits intomainfrom
eb/add-before-exit-hook-and-lots-of-analytics-logging
Apr 10, 2026
Merged

fix: restore beforeExit handler for version commands and add comprehensive telemetry debug logging#3657
eablack merged 2 commits intomainfrom
eb/add-before-exit-hook-and-lots-of-analytics-logging

Conversation

@eablack
Copy link
Copy Markdown
Contributor

@eablack eablack commented Apr 10, 2026

Summary

This PR fixes a regression where telemetry was not being sent for version/help commands, and adds comprehensive debug logging throughout the telemetry system to improve observability.

Key Changes:

  1. Restored beforeExit handler with deduplication mechanism to capture telemetry for commands that don't trigger the postrun hook (e.g., version, --version, -v)
  2. Added comprehensive debug logging that shows telemetry status (enabled/disabled) and the action being taken at each checkpoint
  3. Improved disabled messaging to include the reason for disabling (e.g., DISABLE_TELEMETRY=true, Windows platform requires ENABLE_WINDOWS_TELEMETRY=true)
  4. Optimized redundant checks in bin/run.js to call isTelemetryEnabled() once instead of twice

Problem:

When the telemetry architecture was refactored in commit c13074d, the beforeExit handler was removed to avoid duplicate sends. This caused version/help commands to stop sending telemetry entirely because they don't trigger the postrun hook (oclif handles them specially).

Solution:

  • Added telemetrySent flag to TelemetryGlobal interface
  • Postrun hook sets this flag after sending telemetry
  • beforeExit handler only sends if the flag is NOT set (preventing duplicates)
  • Added contextual debug logging at every telemetry checkpoint

Type of Change

Patch Updates (patch semver update)

  • fix: Bug fix

Testing

Notes:
Set DEBUG=heroku:analytics to see telemetry debug output.

Steps:

  1. Test version command: DEBUG=heroku:analytics heroku version - should see telemetry sent via beforeExit
  2. Test regular command: DEBUG=heroku:analytics heroku apps:info --app myapp - should see telemetry sent via postrun (no duplicate from beforeExit)
  3. Test disabled telemetry: DEBUG=heroku:analytics DISABLE_TELEMETRY=true heroku version - should see clear "disabled" messages with reasons
  4. On Windows without ENABLE_WINDOWS_TELEMETRY=true: Should see appropriate disabled message
  5. Verify no duplicate sends by checking debug output shows only one "Sending telemetry" message per command

Screenshots (if applicable)

N/A - Debug logging output only

Related Issues

https://gus.lightning.force.com/lightning/r/ADM_Work__c/a07EE00002XtZcuYAF/view

@eablack eablack requested a review from a team as a code owner April 10, 2026 17:31
@eablack eablack temporarily deployed to AcceptanceTests April 10, 2026 17:31 — with GitHub Actions Inactive
@eablack eablack temporarily deployed to AcceptanceTests April 10, 2026 17:31 — with GitHub Actions Inactive
@eablack eablack temporarily deployed to AcceptanceTests April 10, 2026 17:31 — with GitHub Actions Inactive
@eablack eablack temporarily deployed to AcceptanceTests April 10, 2026 17:31 — with GitHub Actions Inactive
Copy link
Copy Markdown
Contributor

@tlowrimore-heroku tlowrimore-heroku left a comment

Choose a reason for hiding this comment

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

LGTM!

@eablack eablack temporarily deployed to AcceptanceTests April 10, 2026 19:54 — with GitHub Actions Inactive
@eablack eablack temporarily deployed to AcceptanceTests April 10, 2026 19:54 — with GitHub Actions Inactive
@eablack eablack temporarily deployed to AcceptanceTests April 10, 2026 19:54 — with GitHub Actions Inactive
@eablack eablack temporarily deployed to AcceptanceTests April 10, 2026 19:54 — with GitHub Actions Inactive
@eablack eablack merged commit 6da79cd into main Apr 10, 2026
17 checks passed
@eablack eablack deleted the eb/add-before-exit-hook-and-lots-of-analytics-logging branch April 10, 2026 20:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants