Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Celerdata] Add a grok expression to parse the new log format of BE (ECOINT-101) #2603

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

yandongxiao
Copy link
Contributor

What does this PR do?

Add a grok expression to parse the new log format of BE

Motivation

The newest log format for BE component of StarRocks has changed, so we need to append a new grok expression.

Review checklist

  • PR has a meaningful title or PR has the no-changelog label attached
  • Feature or bugfix has tests
  • Git history is clean
  • If PR impacts documentation, docs team has been notified or an issue has been opened on the documentation repo
  • If this PR includes a log pipeline, please add a description describing the remappers and processors.

Additional Notes

Anything else we should know when reviewing?

@yandongxiao
Copy link
Contributor Author

The failed check reported that I forgot to execute 'ddev validate models celerdata -s'.

Even after I ran ddev validate models celerdata -s on 8.0.0, it still showed errors. By the way, the CelerData plugin was developed using version 8.0.0.

I also tried running ddev validate models celerdata -s with version 11.1.0, but it seems to modify the code content, so as a precaution, I did not submit the following code below.

diff --git a/celerdata/datadog_checks/celerdata/config_models/instance.py b/celerdata/datadog_checks/celerdata/config_models/instance.py
index 3f0792bf..123db35a 100644
--- a/celerdata/datadog_checks/celerdata/config_models/instance.py
+++ b/celerdata/datadog_checks/celerdata/config_models/instance.py
@@ -29,7 +29,7 @@ class AuthToken(BaseModel):
     writer: Optional[MappingProxyType[str, Any]] = None
 
 
-class ExtraMetric(BaseModel):
+class ExtraMetrics(BaseModel):
     model_config = ConfigDict(
         arbitrary_types_allowed=True,
         extra='allow',
@@ -48,7 +48,7 @@ class MetricPatterns(BaseModel):
     include: Optional[tuple[str, ...]] = None
 
 
-class Metric(BaseModel):
+class Metrics(BaseModel):
     model_config = ConfigDict(
         arbitrary_types_allowed=True,
         extra='allow',
@@ -68,7 +68,7 @@ class Proxy(BaseModel):
     no_proxy: Optional[tuple[str, ...]] = None
 
 
-class ShareLabel(BaseModel):
+class ShareLabels(BaseModel):
     model_config = ConfigDict(
         arbitrary_types_allowed=True,
         frozen=True,
@@ -101,7 +101,7 @@ class InstanceConfig(BaseModel):
     exclude_metrics: Optional[tuple[str, ...]] = None
     exclude_metrics_by_labels: Optional[MappingProxyType[str, Union[bool, tuple[str, ...]]]] = None
     extra_headers: Optional[MappingProxyType[str, Any]] = None
-    extra_metrics: Optional[tuple[Union[str, MappingProxyType[str, Union[str, ExtraMetric]]], ...]] = None
+    extra_metrics: Optional[tuple[Union[str, MappingProxyType[str, Union[str, ExtraMetrics]]], ...]] = None
     headers: Optional[MappingProxyType[str, Any]] = None
     histogram_buckets_as_distributions: Optional[bool] = None
     hostname_format: Optional[str] = None
@@ -118,7 +118,7 @@ class InstanceConfig(BaseModel):
     kerberos_principal: Optional[str] = None
     log_requests: Optional[bool] = None
     metric_patterns: Optional[MetricPatterns] = None
-    metrics: Optional[tuple[Union[str, MappingProxyType[str, Union[str, Metric]]], ...]] = None
+    metrics: Optional[tuple[Union[str, MappingProxyType[str, Union[str, Metrics]]], ...]] = None
     min_collection_interval: Optional[float] = None
     namespace: Optional[str] = Field(None, pattern='\\w*')
     non_cumulative_histogram_buckets: Optional[bool] = None
@@ -133,7 +133,7 @@ class InstanceConfig(BaseModel):
     rename_labels: Optional[MappingProxyType[str, Any]] = None
     request_size: Optional[float] = None
     service: Optional[str] = None
-    share_labels: Optional[MappingProxyType[str, Union[bool, ShareLabel]]] = None
+    share_labels: Optional[MappingProxyType[str, Union[bool, ShareLabels]]] = None
     skip_proxy: Optional[bool] = None
     tag_by_endpoint: Optional[bool] = None
     tags: Optional[tuple[str, ...]] = None

@dd-dominic dd-dominic changed the title [Celerdata] Add a grok expression to parse the new log format of BE [Celerdata] Add a grok expression to parse the new log format of BE (ECOINT-101) Feb 14, 2025
Copy link
Contributor

@iliakur iliakur left a comment

Choose a reason for hiding this comment

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

lgtm, feel free to fix the validation. The changes you describe are fine.

@dd-dominic
Copy link
Collaborator

@iliakur can you confirm if the failed validation needs to be fixed in this case?

Looks like @yandongxiao attempted to run the validation command but it was going to modify the code - is this expected?

@iliakur
Copy link
Contributor

iliakur commented Feb 20, 2025

@iliakur can you confirm if the failed validation needs to be fixed in this case?

Looks like @yandongxiao attempted to run the validation command but it was going to modify the code - is this expected?

Yes to both @dd-dominic

@dd-dominic
Copy link
Collaborator

@yandongxiao please run validation command

@yandongxiao
Copy link
Contributor Author

I have tried running ddev validate models celerdata -s with version 11.1.0, and pushed the commit.

@dd-dominic
Copy link
Collaborator

@iliakur The developer ran the validation and committed the changes, but looks like they're still receiving the same error. Any idea what may be causing this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants