Skip to content
Success

Changes

Summary

  1. feat: Add exclusion patterns support to error-log-review CI tool (#3787) (commit: f9dacd2) (details)
Commit f9dacd257266ea0b88227be9429b2d833b5b1b70 by noreply
feat: Add exclusion patterns support to error-log-review CI tool (#3787)

## 🎯 Overview

This PR adds support for exclusion patterns in the error-log-review CI
tool, allowing test code and other non-production paths to be ignored by
specific rules or globally per repository.

## 🐛 Problem

The error-log-review CI was flagging log statements in test files,
creating noise and false positives. For example, in [this PR
run](https://prow.tidb.net/view/gs/prow-tidb-logs/pr-logs/pull/pingcap_ticdc/2337/pull-error-log-review/1976460951846653952):

```
tests/integration_tests/ddl_wait/test.go [string_literals]: log.Fatalf("insert value failed:, host:%s, port:%s, k:%d, i:%d, val:%d, num:%d, err: %+v", host, port, k, i, val, num, err)
tests/integration_tests/ddl_wait/test.go [variables_and_functions]: log.Fatalf("insert value failed:, host:%s, port:%s, k:%d, i:%d, val:%d, num:%d, err: %+v", host, port, k, i, val, num, err)
```

Test code often uses different logging patterns and doesn't require the
same scrutiny as production code.

## ✨ Solution

Implemented a two-level exclusion system using glob patterns:

### 1. Pattern-Specific Excludes
Apply exclusions to individual patterns only:

```yaml
patterns:
  - name: "string_literals"
    regex: '...'
    excludes:
      - "tests/**"           # Exclude all files under tests/
      - "*_test.go"          # Exclude all Go test files
      - "examples/**"        # Exclude examples directory
```

### 2. Repository-Level Excludes
Apply exclusions to all patterns in a repository:

```yaml
repositories:
  - name: "owner/repo"
    excludes:
      - "vendor/**"
      - "third_party/**"
    patterns:
      - name: "..."
```

### Glob Pattern Support
- `*` - matches any sequence of characters in a filename (e.g.,
`*_test.go`)
- `**` - matches directories recursively (e.g., `tests/**` matches all
files under tests/ and subdirectories)
- `?` - matches any single character

## 📝 Changes

### Code Changes
- **`config.go`**: Added `Excludes []string` field to both `Pattern` and
`Repository` structs
- **`config.go`**: Implemented `matchesAnyPattern()` function for glob
pattern matching
- **`matcher.go`**: Updated `CheckPRDiff()` to check exclusions before
applying pattern matching

### Configuration
- **`configs/error-log-review/config.yaml`**: Added `excludes:
["tests/**"]` to `pingcap/ticdc` patterns to fix the reported issue
- **`config.yaml.example`**: Added comprehensive documentation and
examples

### Documentation
- **`README.md`**: Added "Exclusion Patterns" section with detailed
examples and glob syntax reference

### Testing
- **`matcher_test.go`**: Created comprehensive test suite with:
  - `TestMatchesAnyPattern` - Tests glob pattern matching (8 test cases)
  - `TestCheckPRDiffWithExclusions` - Tests end-to-end exclusion logic
  - `TestTiCDCExclusionScenario` - Tests the specific issue scenario
  - All tests passing ✅

## 🔍 Example Usage

```yaml
repositories:
  - name: "pingcap/ticdc"
    patterns:
      - name: "string_literals"
        description: "Pattern for string literals"
        regex: '...'
        excludes:
          - "tests/**"      # Now test files won't trigger this rule
    approvers:
      - "flowbehappy"
```

## ✅ Verification

- All tests pass (3 test suites, 10 test cases)
- Build succeeds
- Code formatted with `go fmt`
- No `go vet` issues
- Configuration validated with YAML parser

## 🎉 Impact

- **Immediate**: Fixes the false positives for `pingcap/ticdc` test
files
- **Future**: Any repository can now configure exclusions without code
changes
- **Flexible**: Supports both global and pattern-specific exclusions
- **Backward Compatible**: Existing configurations continue to work
unchanged

Fixes #[issue_number]

<!-- START COPILOT CODING AGENT SUFFIX -->



<details>

<summary>Original prompt</summary>

>
> ----
>
> *This section details on the original issue you should resolve*
>
> <issue_title>Support exclusion patterns in pull-error-log-review CI
(exclude test code)</issue_title>
> <issue_description>## 🪧 Summary
> Add support for exclusion patterns in the error-log review CI job
(pull-error-log-review) so test code and other non-production paths can
be ignored by specific rules.
>
> ## 🙋 Problem
> Currently, the job flags issues in test files, which creates noise and
false positives. Example from a recent run:
> - tests/integration_tests/ddl_wait/test.go [string_literals]:
log.Fatalf("insert value failed:, host:%s, port:%s, k:%d, i:%d, val:%d,
num:%d, err: %+v", host, port, k, i, val, num, err)
> - tests/integration_tests/ddl_wait/test.go [variables_and_functions]:
log.Fatalf("insert value failed:, host:%s, port:%s, k:%d, i:%d, val:%d,
num:%d, err: %+v", host, port, k, i, val, num, err)
>
> ## Example CI run
>
https://prow.tidb.net/view/gs/prow-tidb-logs/pr-logs/pull/pingcap_ticdc/2337/pull-error-log-review/1976460951846653952
>
> ## 📄 Proposal
> - Add configurable exclusion patterns (e.g., glob paths) so we can
exclude directories like tests/** by default, or on a per-repo basis.
> - Consider supporting both:
>   - Global excludes (apply to all rules), and
> - Rule-specific excludes (e.g., exclude tests/** only for
string_literals and variables_and_functions).
> - Allow repo-level configuration (e.g., a YAML config) to declare
excludes without changing CI code for each repo.
>
> ## 🧐 Acceptance criteria
> - The job supports path-based exclude patterns.
> - Can define global and rule-specific excludes.
> - Document how to configure excludes for a repository.
> - Test files such as tests/integration_tests/** are not flagged by the
above rules once configured.
> </issue_description>
>
> ## Comments on the Issue (you are @copilot in this section)
>
> <comments>
> <comment_new><author>@wuhuizuo</author><body>
> /assign @lybcodes </body></comment_new>
> <comment_new><author>@wuhuizuo</author><body>
> The CI tool information:
> - code path: tools/error-log-review
> - config path:
configs/error-log-review/config.yaml</body></comment_new>
> </comments>
>


</details>

Fixes PingCAP-QE/ci#3786

<!-- START COPILOT CODING AGENT TIPS -->
---

💡 You can make Copilot smarter by setting up custom instructions,
customizing its development environment and configuring Model Context
Protocol (MCP) servers. Learn more [Copilot coding agent
tips](https://gh.io/copilot-coding-agent-tips) in the docs.

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: wuhuizuo <2574558+wuhuizuo@users.noreply.github.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
(commit: f9dacd2)
The file was modifiedtools/error-log-review/config.yaml.example (diff)
The file was modifiedtools/error-log-review/matcher.go (diff)
The file was modifiedconfigs/error-log-review/config.yaml (diff)
The file was modified.gitignore (diff)
The file was modifiedtools/error-log-review/config.go (diff)
The file was modifiedtools/error-log-review/README.md (diff)
The file was addedtools/error-log-review/matcher_test.go