Skip to content

Conversation

@Divkix
Copy link
Contributor

@Divkix Divkix commented Dec 15, 2025

Summary

  • Container name collision fix: Changed SQL queries to use exact matching (=) instead of fuzzy LIKE prefix matching (LIKE ? || '%'). This prevents services with similar names (e.g., myapp and myapp-test) from having their monitoring data incorrectly mixed together.

  • CPU chart Y-axis fix: Made the Y-axis domain dynamic based on actual data values instead of using a fixed [0, 100] range. This allows proper display of CPU usage values >100% which can occur on multi-core systems where Docker reports aggregate CPU usage.

Test plan

  • Deploy two services with overlapping name prefixes (e.g., myapp and myapp-test)
  • Verify monitoring data for each service is isolated and not mixed
  • Verify CPU charts properly display values exceeding 100% (if applicable) without clipping to 0

Fixes #3182

…sues

- Use exact match for container name queries instead of LIKE prefix matching
  to prevent services with similar names (e.g., 'myapp' and 'myapp-test')
  from having their monitoring data mixed together
- Make CPU chart Y-axis domain dynamic to handle values >100% which can
  occur on multi-core systems where Docker reports aggregate CPU usage

Fixes Dokploy#3182
Copilot AI review requested due to automatic review settings December 15, 2025 08:38
@Divkix Divkix requested a review from Siumauricio as a code owner December 15, 2025 08:38
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR attempts to fix two issues: container name collision in monitoring queries and CPU chart Y-axis display problems. However, the SQL query changes introduce a critical bug that will break all metric retrieval.

Key changes:

  • Replaced SQL LIKE prefix matching with exact equality for container name queries
  • Made CPU chart Y-axis domains dynamic based on actual data values
  • Removed string manipulation logic from database query functions

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
apps/monitoring/database/containers.go Changed SQL queries from LIKE to exact match and removed container name suffix-stripping logic - introduces critical bug
apps/dokploy/components/dashboard/monitoring/paid/servers/cpu-chart.tsx Added dynamic Y-axis domain calculation with proper null handling
apps/dokploy/components/dashboard/monitoring/paid/container/container-cpu-chart.tsx Added dynamic Y-axis domain calculation but missing null value safeguards
apps/dokploy/components/dashboard/monitoring/free/container/docker-cpu-chart.tsx Added dynamic Y-axis domain calculation with proper null handling
Comments suppressed due to low confidence (2)

apps/monitoring/database/containers.go:59

  • This change from LIKE prefix matching to exact matching will break metric retrieval. The database stores the full container name (e.g., "myapp-1") via SaveContainerMetric which saves container.Name. However, the appName parameter passed to this function from the API is the service name without the numeric suffix (e.g., "myapp").

In the previous implementation, GetServiceName logic stripped the suffix, allowing "myapp" to match "myapp-1" via LIKE. With exact matching, queries for "myapp" will return no results when the database contains "myapp-1".

To fix the container name collision issue while preserving functionality, consider storing a normalized service name separately in the database or applying the GetServiceName transformation when saving metrics.

func (db *DB) GetLastNContainerMetrics(containerName string, limit int) ([]ContainerMetric, error) {
	query := `
		WITH recent_metrics AS (
			SELECT metrics_json
			FROM container_metrics
			WHERE container_name = ?
			ORDER BY timestamp DESC
			LIMIT ?
		)
		SELECT metrics_json FROM recent_metrics ORDER BY json_extract(metrics_json, '$.timestamp') ASC
	`

apps/monitoring/database/containers.go:92

  • This change from LIKE prefix matching to exact matching will break metric retrieval for the same reason as GetLastNContainerMetrics. The database stores full container names with suffixes (e.g., "myapp-1") but queries use service names without suffixes (e.g., "myapp"), resulting in no matches with exact equality comparison.
func (db *DB) GetAllMetricsContainer(containerName string) ([]ContainerMetric, error) {
	query := `
		WITH recent_metrics AS (
			SELECT metrics_json
			FROM container_metrics
			WHERE container_name = ?
			ORDER BY timestamp DESC
		)
		SELECT metrics_json FROM recent_metrics ORDER BY json_extract(metrics_json, '$.timestamp') ASC
	`

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

timestamp: "",
cpu: 0,
};
const maxCPU = Math.max(...formattedData.map((d) => d.cpu), 100);
Copy link

Copilot AI Dec 15, 2025

Choose a reason for hiding this comment

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

If formattedData contains items where cpu is undefined or null, Math.max will receive these values and may return NaN, causing the Y-axis domain to be invalid. Consider adding a fallback value in the map function to handle missing cpu values, similar to how cpu-chart.tsx uses d.cpu || 0.

Suggested change
const maxCPU = Math.max(...formattedData.map((d) => d.cpu), 100);
const maxCPU = Math.max(...formattedData.map((d) => d.cpu || 0), 100);

Copilot uses AI. Check for mistakes.
… null guard

- Pass serviceName to processContainerMetrics() and use it for metric Name field
  instead of container.Name, ensuring exact match queries work correctly
- Add || 0 fallback in container-cpu-chart.tsx to prevent NaN in Y-axis domain
  calculation when cpu values are null/undefined

Addresses PR review feedback on incomplete SQL query fix and missing null handling.
@Divkix
Copy link
Contributor Author

Divkix commented Dec 15, 2025

Addressed Review Feedback

Fixed both issues identified in the review:

1. SQL Query Fix (Critical)

The original change from LIKE to exact match was incomplete. The issue was that metrics were stored with full container names (e.g., myapp-1) but queries used service names (e.g., myapp), causing exact match to return no results.

Fix: Updated processContainerMetrics() to accept and use serviceName instead of container.Name when storing metrics. Now data is stored with the stripped service name, making exact match queries work correctly.

Changes in apps/monitoring/containers/monitor.go:

  • Added serviceName parameter to processContainerMetrics()
  • Changed Name: container.NameName: serviceName
  • Updated call site to pass serviceName

2. Null Handling Fix

Added the missing || 0 fallback in container-cpu-chart.tsx to prevent NaN when cpu values are null/undefined, matching the pattern used in the other chart components.

Change in apps/dokploy/components/dashboard/monitoring/paid/container/container-cpu-chart.tsx:

- const maxCPU = Math.max(...formattedData.map((d) => d.cpu), 100);
+ const maxCPU = Math.max(...formattedData.map((d) => d.cpu || 0), 100);

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.

Monitor problem

1 participant