refactoring to allow for slack error posting

[MartyEwings]

This pull request refactors the metrics import script to add Slack error notification capabilities. The changes introduce a configurable Slack webhook integration while maintaining the existing file processing and metrics import functionality.

Key changes:

Added slack_post() and log() helper functions to enable Slack notifications
Introduced configuration section for Slack webhook settings
Replaced direct echo statements with log() calls to route messages to both stdout and Slack

[Copilot]

Pull request overview

This pull request refactors the metrics import script to add Slack error notification capabilities. The changes introduce a configurable Slack webhook integration while maintaining the existing file processing and metrics import functionality.

Key changes:

• Added slack_post() and log() helper functions to enable Slack notifications
• Introduced configuration section for Slack webhook settings
• Replaced direct echo statements with log() calls to route messages to both stdout and Slack

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The slack_post function lacks documentation explaining its parameters, return behavior, and the fact that it silently returns success (0) when SEND_SLACK is false or SLACK_WEBHOOK_URL is empty. Adding a brief comment would improve maintainability.

Suggested change

	slack_post() {
	# Usage: slack_post "message"
	slack_post() {
	# Post a message to Slack.
	# Usage: slack_post "message"
	# Params:
	# $1 - message text to send.
	# Behavior / return:
	# - If SEND_SLACK is not "true" or SLACK_WEBHOOK_URL is empty, the function
	# returns success (exit code 0) without sending anything.
	# - Network or curl errors are ignored so callers should not rely on this
	# function's exit status to detect delivery failures.

[Copilot]

The JSON escaping implementation is incomplete and could lead to malformed JSON. The current implementation only escapes backslashes, double-quotes, and newlines, but does not handle other JSON control characters such as tabs, carriage returns, backspaces, and form feeds. These characters could break the JSON payload if present in error messages.

Suggested change

	# Minimal JSON escaping for double-quotes, plus newlines -> \n
	local esc="${msg//\\/\\\\}"
	esc="${esc//\"/\\\"}"
	esc="${esc//$'\n'/\\n}"
	# Minimal JSON escaping for backslashes, double-quotes, and control chars
	local esc="${msg//\\/\\\\}"
	esc="${esc//\"/\\\"}"
	esc="${esc//$'\n'/\\n}"
	esc="${esc//$'\r'/\\r}"
	esc="${esc//$'\t'/\\t}"
	esc="${esc//$'\b'/\\b}"
	esc="${esc//$'\f'/\\f}"

[Copilot]

The curl command's error output is suppressed with '2>&1 || true', which means failures to post to Slack will be silently ignored. While this prevents the script from failing when Slack is unavailable, it provides no feedback about posting failures, making it difficult to diagnose connectivity or webhook configuration issues.

Suggested change

	curl -sS -X POST -H 'Content-type: application/json' \
	--data "{\"text\":\"$esc\"}" \
	"$SLACK_WEBHOOK_URL" >/dev/null 2>&1 || true
	if ! curl -sS -X POST -H 'Content-type: application/json' \
	--data "{\"text\":\"$esc\"}" \
	"$SLACK_WEBHOOK_URL" >/dev/null; then
	echo "WARN: failed to post message to Slack webhook" >&2
	fi

[Copilot]

The SLACK_WEBHOOK_URL is read from an environment variable but there's no validation that it's set when SEND_SLACK is true. If SEND_SLACK is true but SLACK_WEBHOOK_URL is not set, slack_post will silently do nothing. Consider adding validation at startup to fail early with a clear error message if SEND_SLACK is true but SLACK_WEBHOOK_URL is empty.

Suggested change

	# Validate Slack configuration early
	if [[ "$SEND_SLACK" == "true" && -z "$SLACK_WEBHOOK_URL" ]]; then
	echo "ERROR: SEND_SLACK is true but SLACK_WEBHOOK_URL is not set. Please export SLACK_WEBHOOK_URL or set SEND_SLACK=false." >&2
	exit 1
	fi

[Copilot]

Not all error paths are using the new log() function. The "No metrics found" message at line 75 is informational rather than an error, but other error conditions use log() with "ERROR:" prefix. For consistency, consider whether this message should also be treated as an error or if the current behavior is intentional.

Suggested change

	log "No metrics found in $f. Skipping"
	log "INFO: No metrics found in $f. Skipping"

[dnecco]

LGTM