Skip to content

Conversation

eventhorizon-cli
Copy link
Contributor

No description provided.

@eventhorizon-cli eventhorizon-cli requested a review from Copilot April 1, 2025 08:13
Copy link

@Copilot 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 pull request implements metrics support by adding new extension methods for statistical calculations and enum descriptions, updating documentation for both Chinese and English quick-start guides, and enhancing the docker-compose configuration to include metrics storage using InfluxDB.

  • Added EnumerableExtensions for computing variance and standard deviation
  • Added EnumExtensions for retrieving enum descriptions with caching
  • Updated documentation and docker-compose.yml to integrate metrics support via InfluxDB and a newer Grafana version

Reviewed Changes

Copilot reviewed 164 out of 171 changed files in this pull request and generated no comments.

Show a summary per file
File Description
src/Mocha.Core/Extensions/EnumerableExtensions.cs Provides new methods to calculate population variance and standard deviation from sequence data
src/Mocha.Core/Extensions/EnumExtensions.cs Adds extension methods to handle enum descriptions with caching for performance optimization
docs/quick-start/docker-compose/quick-start.zh-CN.md Updates Docker Compose instructions and asset paths for the Chinese quick-start guide
docs/quick-start/docker-compose/quick-start.en-US.md Updates Docker Compose instructions and asset paths for the English quick-start guide
docker/docker-compose.yml Introduces an InfluxDB service for metrics storage and updates service configurations and Grafana version
Files not reviewed (7)
  • Mocha.sln: Language not supported
  • docker/query/Dockerfile: Language not supported
  • scripts/mysql/init/metadata.sql: Language not supported
  • scripts/mysql/init/trace.sql: Language not supported
  • src/Mocha.Antlr4.Generated/Mocha.Antlr4.Generated.csproj: Language not supported
  • src/Mocha.Antlr4.Generated/PromoQL/PromQLLexer.g4: Language not supported
  • src/Mocha.Antlr4.Generated/PromoQL/PromQLParser.g4: Language not supported
Comments suppressed due to low confidence (1)

docker/docker-compose.yml:1

  • The removal of the 'version' field in the docker-compose file may lead to compatibility issues with certain Docker Compose versions; please verify that the updated file syntax is compatible with your deployment environment.
version: "3.8"

Copy link

@Copilot 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.

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

Files not reviewed (7)
  • Mocha.sln: Language not supported
  • docker/query/Dockerfile: Language not supported
  • scripts/mysql/init/metadata.sql: Language not supported
  • scripts/mysql/init/trace.sql: Language not supported
  • src/Mocha.Antlr4.Generated/Mocha.Antlr4.Generated.csproj: Language not supported
  • src/Mocha.Antlr4.Generated/PromoQL/PromQLLexer.g4: Language not supported
  • src/Mocha.Antlr4.Generated/PromoQL/PromQLParser.g4: Language not supported

}

var mean = sum / count;
return sumSq / count - mean * mean;
Copy link

Copilot AI Apr 7, 2025

Choose a reason for hiding this comment

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

The variance calculation may produce a slightly negative result due to floating point rounding errors, which causes Math.Sqrt to return NaN. Consider using Math.Max(0, sumSq / count - mean * mean) to ensure a non-negative variance.

Suggested change
return sumSq / count - mean * mean;
return Math.Max(0, sumSq / count - mean * mean);

Copilot uses AI. Check for mistakes.

@eventhorizon-cli eventhorizon-cli requested a review from Copilot April 7, 2025 12:59
Copy link

@Copilot 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.

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

Files not reviewed (7)
  • Mocha.sln: Language not supported
  • docker/query/Dockerfile: Language not supported
  • scripts/mysql/init/metadata.sql: Language not supported
  • scripts/mysql/init/trace.sql: Language not supported
  • src/Mocha.Antlr4.Generated/Mocha.Antlr4.Generated.csproj: Language not supported
  • src/Mocha.Antlr4.Generated/PromoQL/PromQLLexer.g4: Language not supported
  • src/Mocha.Antlr4.Generated/PromoQL/PromQLParser.g4: Language not supported
Comments suppressed due to low confidence (1)

docker/docker-compose.yml:22

  • The volume binding has changed from a single initialization file to a directory; please ensure that the entrypoint script is configured to process multiple initialization files if this change is intentional.
-      - ../scripts/mysql/init:/docker-entrypoint-initdb.d/

Comment on lines +25 to +26
var mean = sum / count;
return Math.Max(0, sumSq / count - mean * mean);
Copy link

Copilot AI Apr 7, 2025

Choose a reason for hiding this comment

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

[nitpick] Clamping the variance to 0 using Math.Max may mask underlying numerical precision issues. Consider documenting the rationale or using a more numerically stable algorithm (e.g., Welford's algorithm) if high precision is required.

Suggested change
var mean = sum / count;
return Math.Max(0, sumSq / count - mean * mean);
return m2 / count;

Copilot uses AI. Check for mistakes.

Copy link

codecov bot commented May 16, 2025

Codecov Report

Attention: Patch coverage is 17.36018% with 1847 lines in your changes missing coverage. Please review.

Project coverage is 38.67%. Comparing base (8bdc420) to head (7a9fb4d).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
.../Mocha.Query/Prometheus/PromQL/Engine/Evaluator.cs 0.00% 543 Missing ⚠️
...s/Metrics/OTelToMochaMetricConversionExtensions.cs 0.00% 349 Missing ⚠️
.../Mocha.Query/Prometheus/PromQL/Engine/Functions.cs 0.00% 148 Missing ⚠️
...ery/Prometheus/Controllers/PrometheusController.cs 0.00% 122 Missing ⚠️
...aders/Prometheus/InfluxDBPrometheusMetricReader.cs 0.00% 109 Missing ⚠️
...cha.Query/Prometheus/PromQL/Engine/PromQLEngine.cs 0.00% 88 Missing ⚠️
...rc/Mocha.Query/Prometheus/PromQL/Ast/AstBuilder.cs 65.40% 61 Missing and 12 partials ⚠️
src/Mocha.Core/Storage/Prometheus/Labels.cs 8.33% 44 Missing ⚠️
src/Mocha.Query/Program.cs 0.00% 32 Missing ⚠️
...tadata/Readers/EFPrometheusMetricMetadataReader.cs 0.00% 28 Missing ⚠️
... and 45 more

❗ There is a different number of reports uploaded between BASE (8bdc420) and HEAD (7a9fb4d). Click for more details.

HEAD has 1 upload less than BASE
Flag BASE (8bdc420) HEAD (7a9fb4d)
2 1
Additional details and impacted files
@@             Coverage Diff             @@
##             main      #42       +/-   ##
===========================================
- Coverage   91.64%   38.67%   -52.97%     
===========================================
  Files          51      123       +72     
  Lines         993     3294     +2301     
  Branches      107      449      +342     
===========================================
+ Hits          910     1274      +364     
- Misses         57     1971     +1914     
- Partials       26       49       +23     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@liuhaoyang liuhaoyang merged commit bf4edd5 into dotnetcore:main May 18, 2025
2 of 4 checks passed
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