Skip to content

Commit

Permalink
Validate that telemetry definitions are ordered alphabetically; Reord…
Browse files Browse the repository at this point in the history
…er the current file (#705)

## Problem

Metrics are defined in haphazard order in the telemetry definitions file. This makes it challenging to locate entries, particularly those sharing a common prefix. Maintaining sorted lists also makes it easier to reason about entries being added or modified, without having to think about their location.

## Solution

Scripting has been created to:
- test that the definition file is arranged in alphabetical order
- automatically rearrange the file's entries in alphabetical order

A CI job has been set up to run the validation check during PRs

This change was not wired up as a commit hook, because the repo does not have a top level node project. The CI job, and the readme are the mechanisms used to maintain the file going forward.

I chose not to use eslint. I started out in that direction, but it was becoming more of a distraction from the problem being solved. From https://github.com/azeemba/eslint-plugin-json?tab=readme-ov-file#is-eslint-really-the-best-tool-to-lint-my-json:
> Is **eslint** really the best tool to lint my JSON?
> Not really. `eslint` plugin interface wasn't designed to lint a completely different language but its interface is flexible enough to allow it. So this plugin is certainly unusual.

I ran the fix script against the current definitions file. This PR also contains that change as a separate commit.

To verify this change, I ran the new fix script on the current telemetry file. I then hand sorted the same file, and verified that the resulting file matched what the script produced.
  • Loading branch information
awschristou authored Mar 6, 2024
1 parent c07e6a5 commit ec38b82
Show file tree
Hide file tree
Showing 12 changed files with 8,504 additions and 2,533 deletions.
26 changes: 26 additions & 0 deletions .github/workflows/lint-telemetry-definitions.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
name: Lint Telemetry Definitions
on:
push:
branches: [main]
pull_request:
branches: [main, feature/*]

jobs:
build:
name: Build and Test
runs-on: ubuntu-latest
steps:
- name: Sync Code
uses: actions/checkout@v3

- name: Compile the Validation code
run: |
cd telemetry/validation
npm ci
npm run build
npm run test
- name: Validate the telemetry definitions
run: |
cd telemetry/validation
npm run validate
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ build
.gradle
.idea
**/node_modules
**/dist
**/out
**/*.tgz
.testresults/**
Expand Down
10 changes: 10 additions & 0 deletions telemetry/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,17 @@ and change `name`, `description`, and the values in `allowedValues`

Then rerun the generator. This example will generate a new type that can be imported or used in telemetry calls.

CI jobs run validation checks on the telemetry definitions file. For example, entries are expected to exist in alphabetical order. You may want to check this yourself before creating a PR, by switching to the `telemetry/validation` folder, and running:

```
npm install
npm run validate
```

You can also run `npm run fix` to apply simple fixes to the file (like re-ordering).

### Overriding existing telemetry

In VSCode and JetBrains, extra telemetry files will take precedence over existing definitions. For example, if you have a metric for `lambda_update`, adding another `lambda_update` in the repo's extra telemetry files will override all of the values of it.

Types work similarly, and will also be overwritten by extra telemetry files.
Expand Down
Loading

0 comments on commit ec38b82

Please sign in to comment.