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

generate types, create http client and add exemplary usage #3384

Merged
merged 25 commits into from
Nov 29, 2023
Merged
Show file tree
Hide file tree
Changes from 22 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions .pre-commit-config.yaml
Copy link
Contributor

Choose a reason for hiding this comment

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

nit it seems most of our .yaml files use double-quotes. We might want to consider configuring the quoted-strings rule to make things consistent

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I switched to double quotes in this exact file. But across the codebase we have a mix now so yes, maybe in a separate PR we could change everything to double quotes and add a rule to yamllint

Original file line number Diff line number Diff line change
Expand Up @@ -41,29 +41,29 @@ repos:
files: ^tools/pagerduty-migrator
# Make sure config is compatible with black
# https://black.readthedocs.io/en/stable/guides/using_black_with_other_tools.html#flake8
args: ["--max-line-length=88", "--extend-ignore=E203,E501"]
args: ['--max-line-length=88', '--extend-ignore=E203,E501']
- id: flake8
name: flake8 - dev/scripts
files: ^dev/scripts
# Make sure config is compatible with black
# https://black.readthedocs.io/en/stable/guides/using_black_with_other_tools.html#flake8
args: ["--max-line-length=88", "--extend-ignore=E203,E501"]
args: ['--max-line-length=88', '--extend-ignore=E203,E501']

- repo: https://github.com/pre-commit/mirrors-eslint
rev: v8.25.0
hooks:
- id: eslint
entry: bash -c 'cd grafana-plugin && eslint --max-warnings=0 --fix ${@/grafana-plugin\//}' --
types: [file]
files: ^grafana-plugin/src/.*\.(js|jsx|ts|tsx)$
files: ^grafana-plugin/src/(?:(?!autogenerated).)*\.(js|jsx|ts|tsx)$
additional_dependencies:
- eslint@^8.25.0
- eslint-plugin-import@^2.25.4
- eslint-plugin-rulesdir@^0.2.1
- "@grafana/eslint-config@^5.0.0"
- '@grafana/eslint-config@^5.0.0'

- repo: https://github.com/pre-commit/mirrors-prettier
rev: "v2.7.1"
rev: 'v2.7.1'
hooks:
- id: prettier
name: prettier
Expand Down
2 changes: 2 additions & 0 deletions dev/.env.dev.example
Original file line number Diff line number Diff line change
Expand Up @@ -41,3 +41,5 @@ RABBITMQ_PORT=5672
RABBITMQ_DEFAULT_VHOST="/"

REDIS_URI=redis://redis:6379/0

DRF_SPECTACULAR_ENABLED=False
joeyorlando marked this conversation as resolved.
Show resolved Hide resolved
26 changes: 17 additions & 9 deletions dev/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -43,25 +43,33 @@ Related: [How to develop integrations](/engine/config_integrations/README.md)

1. Create local k8s cluster:

```bash
make cluster/up
```
```bash
make cluster/up
```

2. Deploy the project:

```bash
tilt up
```
```bash
tilt up
```

You can set local environment variables using `dev/helm-local.dev.yml` file, e.g.:

```yaml
env:
- name: FEATURE_LABELS_ENABLED_FOR_ALL
value: 'True'
```

3. Wait until all resources are green and open <http://localhost:3000/a/grafana-oncall-app> (user: oncall, password: oncall)

4. Modify source code, backend and frontend will be hot reloaded

5. Clean up the project by deleting the local k8s cluster:

```bash
make cluster/down
```
```bash
make cluster/down
```

## Running the project with docker-compose

Expand Down
4 changes: 2 additions & 2 deletions engine/engine/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -84,9 +84,9 @@
]

if settings.DRF_SPECTACULAR_ENABLED:
from drf_spectacular.views import SpectacularAPIView, SpectacularSwaggerView
from drf_spectacular.views import SpectacularSwaggerView, SpectacularYAMLAPIView

urlpatterns += [
path("internal/schema/", SpectacularAPIView.as_view(api_version="internal/v1"), name="schema"),
path("internal/schema/", SpectacularYAMLAPIView.as_view(api_version="internal/v1"), name="schema"),
path("internal/schema/swagger-ui/", SpectacularSwaggerView.as_view(url_name="schema"), name="swagger-ui"),
]
3 changes: 2 additions & 1 deletion grafana-plugin/.eslintignore
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
/src/assets
/src/assets
/src/network/oncall-api/autogenerated-api.types.d.ts
89 changes: 89 additions & 0 deletions grafana-plugin/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
# Grafana OnCall

Developer-Friendly
Alert Management
with Brilliant Slack Integration

- Connect monitoring systems
- Collect and analyze data
- On-call rotation
- Automatic escalation
- Never miss alerts with calls and SMS

## Documentation

- [On Github](http://github.com/grafana/oncall)
- [Grafana OnCall](https://grafana.com/docs/oncall/latest/)

## Development

### Autogenerating TS types based on OpenAPI schema

| :warning: WARNING |
| :------------------------------------------------------------------------------------------ |
| Transition to this approach is [in progress](https://github.com/grafana/oncall/issues/3338) |
Copy link
Contributor

Choose a reason for hiding this comment

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

<3


#### Overview

In order to automate types creation and prevent API usage pitfalls, OnCall project is using the following approach:

1. OnCall Engine (backend) exposes OpenAPI schema
2. OnCall Grafana Plugin (frontend) autogenerates TS type definitions based on it
3. OnCall Grafana Plugin (frontend) uses autogenerated types as a single source of truth for
any backend-related interactions (url paths, request bodies, params, response payloads)

#### Instruction

1. Whenever API contract changes, run `yarn generate-types` from `grafana-plugin` directory
2. Then you can start consuming types and you can use fully typed http client:

```ts
import { ApiSchemas } from 'network/oncall-api/api.types';
import onCallApi from 'network/oncall-api/http-client';

const {
data: { results },
} = await onCallApi.GET('/alertgroups/');
const alertGroups: Array<ApiSchemas['AlertGroup']> = results;
```

3. [Optional] If there is any property that is not yet exposed in OpenAPI schema and you already want to use it,
you can append missing properties to particular schemas by editing
`grafana-plugin/src/network/oncall-api/types-generator/custom-schemas.ts` file:

```ts
export type CustomApiSchemas = {
Alert: {
propertyMissingInOpenAPI: string;
};
AlertGroup: {
anotherPropertyMissingInOpenAPI: number[];
};
};
```

Then add their names to `CUSTOMIZED_SCHEMAS` array in `grafana-plugin/src/network/oncall-api/types-generator/generate-types.ts`:

```ts
const CUSTOMIZED_SCHEMAS = ['Alert', 'AlertGroup'];
```

The outcome is that autogenerated schemas will be modified as follows:

```ts
import type { CustomApiSchemas } from './types-generator/custom-schemas';

export interface components {
schemas: {
Alert: CustomApiSchemas['Alert'] & {
readonly id: string;
...
};
AlertGroup: CustomApiSchemas['AlertGroup'] & {
readonly pk: string;
...
},
...
}
}
```
5 changes: 3 additions & 2 deletions grafana-plugin/jest.config.js
Original file line number Diff line number Diff line change
@@ -1,15 +1,16 @@
const esModules = ['@grafana', 'uplot', 'ol', 'd3', 'react-colorful', 'uuid'].join('|');
const esModules = ['@grafana', 'uplot', 'ol', 'd3', 'react-colorful', 'uuid', 'openapi-fetch'].join('|');

module.exports = {
testEnvironment: 'jsdom',

moduleDirectories: ['node_modules', 'src'],
moduleFileExtensions: ['ts', 'tsx', 'js'],
moduleFileExtensions: ['ts', 'tsx', 'js', 'd.ts', 'cjs'],

transformIgnorePatterns: [`/node_modules/(?!${esModules})`],

moduleNameMapper: {
'grafana/app/(.*)': '<rootDir>/src/jest/grafanaMock.ts',
'openapi-fetch': '<rootDir>/src/jest/openapiFetchMock.ts',
'jest/matchMedia': '<rootDir>/src/jest/matchMedia.ts',
'^jest$': '<rootDir>/src/jest',
'^.+\\.(css|scss)$': '<rootDir>/src/jest/styleMock.ts',
Expand Down
5 changes: 4 additions & 1 deletion grafana-plugin/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@
"test:e2e-expensive": "yarn playwright test --grep @expensive",
"test:e2e:watch": "yarn test:e2e --ui",
"test:e2e:gen": "yarn playwright codegen http://localhost:3000",
"cleanup-e2e-results": "rm -rf playwright-report && rm -rf test-results",
"e2e-show-report": "yarn playwright show-report",
"generate-types": "cd ./src/network/oncall-api/types-generator && yarn generate",
"dev": "grafana-toolkit plugin:dev",
"watch": "grafana-toolkit plugin:dev --watch",
"sign": "grafana-toolkit plugin:sign",
Expand Down Expand Up @@ -96,6 +96,7 @@
"lodash-es": "^4.17.21",
"mailslurp-client": "^15.14.1",
"moment-timezone": "^0.5.35",
"openapi-typescript": "^7.0.0-next.4",
"plop": "^2.7.4",
"postcss-loader": "^7.0.1",
"react": "17.0.2",
Expand All @@ -105,6 +106,7 @@
"stylelint-prettier": "^2.0.0",
"ts-jest": "29.0.3",
"ts-loader": "^9.3.1",
"ts-node": "^10.9.1",
"typescript": "4.6.4",
"webpack-bundle-analyzer": "^4.6.1"
},
Expand All @@ -127,6 +129,7 @@
"mobx": "5.13.0",
"mobx-react": "6.1.1",
"object-hash": "^3.0.0",
"openapi-fetch": "^0.8.1",
"prettier": "^2.8.2",
"qrcode.react": "^3.1.0",
"raw-loader": "^4.0.2",
Expand Down
16 changes: 0 additions & 16 deletions grafana-plugin/src/README.md

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { observer } from 'mobx-react';

import Text from 'components/Text/Text';
import { AlertReceiveChannel } from 'models/alert_receive_channel/alert_receive_channel.types';
import { LabelKey } from 'models/label/label.types';
import { ApiSchemas } from 'network/oncall-api/api.types';
import { useStore } from 'state/useStore';

import styles from './IntegrationLabelsForm.module.css';
Expand Down Expand Up @@ -45,7 +45,7 @@ const IntegrationLabelsForm = observer((props: IntegrationLabelsFormProps) => {
onOpenIntegraionSettings(id);
};

const getInheritanceChangeHandler = (keyId: LabelKey['id']) => {
const getInheritanceChangeHandler = (keyId: ApiSchemas['LabelKey']['id']) => {
return (event: React.ChangeEvent<HTMLInputElement>) => {
setAlertGroupLabels((alertGroupLabels) => ({
...alertGroupLabels,
Expand Down
1 change: 1 addition & 0 deletions grafana-plugin/src/jest/openapiFetchMock.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export default () => ({});
12 changes: 2 additions & 10 deletions grafana-plugin/src/models/alertgroup/alertgroup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@ import qs from 'query-string';

import { AlertReceiveChannel } from 'models/alert_receive_channel/alert_receive_channel.types';
import BaseStore from 'models/base_store';
import { LabelKey } from 'models/label/label.types';
import { User } from 'models/user/user.types';
import { makeRequest } from 'network';
import { ApiSchemas } from 'network/oncall-api/api.types';
import { Mixpanel } from 'services/mixpanel';
import { RootStore } from 'state';
import { SelectOption } from 'state/types';
Expand Down Expand Up @@ -63,9 +63,6 @@ export class AlertGroupStore extends BaseStore {
@observable
silencedIncidents: any = {};

@observable
alertGroupStats: any = {};

@observable
liveUpdatesEnabled = false;

Expand Down Expand Up @@ -358,11 +355,6 @@ export class AlertGroupStore extends BaseStore {
this.silencedIncidents = result;
}

@action
async getAlertGroupsStats() {
this.alertGroupStats = await makeRequest('/alertgroups/stats/', {});
}

@action
async doIncidentAction(alertId: Alert['pk'], action: AlertAction, isUndo = false, data?: any) {
this.updateAlert(alertId, { loading: true });
Expand Down Expand Up @@ -442,7 +434,7 @@ export class AlertGroupStore extends BaseStore {
}

@action
public async loadValuesForLabelKey(key: LabelKey['id'], search = '') {
public async loadValuesForLabelKey(key: ApiSchemas['LabelKey']['id'], search = '') {
if (!key) {
return [];
}
Expand Down
Loading
Loading