diff options
Diffstat (limited to 'doc/development')
23 files changed, 463 insertions, 59 deletions
diff --git a/doc/development/architecture.md b/doc/development/architecture.md index e22552fd3a3..ae880e3deb6 100644 --- a/doc/development/architecture.md +++ b/doc/development/architecture.md @@ -65,14 +65,14 @@ Gitaly is a service designed by GitLab to remove our need for NFS for Git storag - Omnibus configuration options - Layer: Monitoring -GitLab Monitor is a process disigned in house that allows us to export metrics about GitLab application internals to prometheus. You can read more [in the project's readme](https://gitlab.com/gitlab-org/gitlab-monitor) +GitLab Monitor is a process designed in house that allows us to export metrics about GitLab application internals to prometheus. You can read more [in the project's readme](https://gitlab.com/gitlab-org/gitlab-monitor) ### gitlab-workhorse - Omnibus configuration options - Layer: Core Service (Processor) -[GitLab Workhorse](https://gitlab.com/gitlab-org/gitlab-workhorse) is a program designed at GitLab to help alieviate pressure from unicorn. You can read more about the [historical reasons for developing](https://about.gitlab.com/2016/04/12/a-brief-history-of-gitlab-workhorse/). It's designed to act as a smart reverse proxy to help speed up GitLab as a whole. +[GitLab Workhorse](https://gitlab.com/gitlab-org/gitlab-workhorse) is a program designed at GitLab to help alleviate pressure from unicorn. You can read more about the [historical reasons for developing](https://about.gitlab.com/2016/04/12/a-brief-history-of-gitlab-workhorse/). It's designed to act as a smart reverse proxy to help speed up GitLab as a whole. ### logrotate diff --git a/doc/development/changelog.md b/doc/development/changelog.md index cd0a1f46d27..6efed36edf0 100644 --- a/doc/development/changelog.md +++ b/doc/development/changelog.md @@ -135,21 +135,13 @@ If you're working on the GitLab EE repository, the entry will be added to | Argument | Shorthand | Purpose | | ----------------- | --------- | --------------------------------------------------------------------------------------------------------------------------------------- | -| [`--amend`] | | Amend the previous commit | -| [`--force`] | `-f` | Overwrite an existing entry | -| [`--merge-request`] | `-m` | Set merge request ID | -| [`--dry-run`] | `-n` | Don't actually write anything, just print | -| [`--git-username`] | `-u` | Use Git user.name configuration as the author | -| [`--type`] | `-t` | The category of the change, valid options are: `added`, `fixed`, `changed`, `deprecated`, `removed`, `security`, `performance`, `other` | -| [`--help`] | `-h` | Print help message | - -[`--amend`]: #-amend -[`--force`]: #-force-or-f -[`--merge-request`]: #-merge-request-or-m -[`--dry-run`]: #-dry-run-or-n -[`--git-username`]: #-git-username-or-u -[`--type`]: #-type-or-t -[`--help`]: #-help +| [`--amend`](#--amend) | | Amend the previous commit | +| [`--force`](#--force-or--f) | `-f` | Overwrite an existing entry | +| [`--merge-request`](#--merge-request-or--m) | `-m` | Set merge request ID | +| [`--dry-run`](#--dry-run-or--n) | `-n` | Don't actually write anything, just print | +| [`--git-username`](#--git-username-or--u) | `-u` | Use Git user.name configuration as the author | +| [`--type`](#--type-or--t) | `-t` | The category of the change, valid options are: `added`, `fixed`, `changed`, `deprecated`, `removed`, `security`, `performance`, `other` | +| `--help` | `-h` | Print help message | ##### `--amend` diff --git a/doc/development/contributing/merge_request_workflow.md b/doc/development/contributing/merge_request_workflow.md index 4e766f37871..8b14c3b20ea 100644 --- a/doc/development/contributing/merge_request_workflow.md +++ b/doc/development/contributing/merge_request_workflow.md @@ -179,7 +179,7 @@ the feature you contribute through all of these steps. 1. Added to [the website](https://gitlab.com/gitlab-com/www-gitlab-com/), if relevant 1. Community questions answered 1. Answers to questions radiated (in docs/wiki/support etc.) -1. [Black-box tests/end-to-end tests](../testing_guide/testing_levels.md#black-box-tests-or-end-to-end-tests) added if required. Please contact [the quality team](https://about.gitlab.com/handbook/engineering/quality/#teams) with any questions +1. [Black-box tests/end-to-end tests](../testing_guide/testing_levels.md#black-box-tests-at-the-system-level-aka-end-to-end-tests) added if required. Please contact [the quality team](https://about.gitlab.com/handbook/engineering/quality/#teams) with any questions If you add a dependency in GitLab (such as an operating system package) please consider updating the following and note the applicability of each in your diff --git a/doc/development/distributed_tracing.md b/doc/development/distributed_tracing.md new file mode 100644 index 00000000000..038e3de10d7 --- /dev/null +++ b/doc/development/distributed_tracing.md @@ -0,0 +1,182 @@ +# Distributed Tracing - development guidelines + +GitLab is instrumented for distributed tracing. + +According to [Open Tracing](https://opentracing.io/docs/overview/what-is-tracing/): + +> Distributed tracing, also called distributed request tracing, is a method used to profile and +> monitor applications, especially those built using a microservices architecture. Distributed +> tracing helps to pinpoint where failures occur and what causes poor performance. + +Distributed tracing is especially helpful in understanding the lifecycle of a request as it passes +through the different components of the GitLab application. At present, Workhorse, Rails, Sidekiq, +and Gitaly support tracing instrumentation. + +Distributed tracing adds minimal overhead when disabled, but imposes only small overhead when +enabled and is therefore capable in any environment, including production. For this reason, it can +be useful in diagnosing production issues, particularly performance problems. + +## Enabling distributed tracing + +GitLab uses the `GITLAB_TRACING` environment variable to configure distributed tracing. The same +configuration is used for all components (e.g., Workhorse, Rails, etc). + +When `GITLAB_TRACING` is not set, the application will not be instrumented, meaning that there is +no overhead at all. + +To enable `GITLAB_TRACING`, a valid _"configuration-string"_ value should be set, with a URL-like +form: + +```console +GITLAB_TRACING=opentracing://<driver>?<param_name>=<param_value>&<param_name_2>=<param_value_2> +``` + +In this example, we have the following hypothetical values: + +- `driver`: the driver. [GitLab supports + `jaeger`](https://docs.gitlab.com/ee/user/project/operations/tracing.html). In future, other + tracing implementations may also be supported. +- `param_name`, `param_value`: these are driver specific configuration values. Configuration + parameters for Jaeger are documented [further on in this + document](#2-configure-the-gitlab_tracing-environment-variable) they should be URL encoded. + Multiple values should be separated by `&` characters like a URL. + +## Using Jaeger in the GitLab Development Kit + +The first tracing implementation that GitLab supports is Jaeger, and the [GitLab Development +Kit](https://gitlab.com/gitlab-org/gitlab-development-kit/) supports distributed tracing with +Jaeger out-of-the-box. + +The easiest way to access tracing from a GDK environment is through the +[performance-bar](../administration/monitoring/performance/performance_bar.md). This can be shown +by typing `p` `b` in the browser window. + + + +Once the performance bar is enabled, click on the **Trace** link in the performance bar to go to +the Jaeger UI. + +The Jaeger search UI will return a query for the `Correlation-ID` of the current request. Normally, +this search should return a single trace result. Clicking this result will show the detail of the +trace in a hierarchical time-line. + + + +## Using Jaeger without the GitLab Developer Kit + +Distributed Tracing can be enabled in non-GDK development environments as well as production or +staging environments, for troubleshooting. Please note that at this time, this functionality is +experimental, and not supported in production environments at present. In this first release, it is intended to be +used for debugging in development environments only. + +Jaeger tracing can be enabled through a three-step process: + +1. [Start Jaeger](#1-start-jaeger). +1. [Configure the `GITLAB_TRACING` environment variable](#2-configure-the-gitlab_tracing-environment-variable). +1. [Start the GitLab application](#3-start-the-gitlab-application). +1. [Go to the Jaeger Search UI in your browser](#4-open-the-jaeger-search-ui). + +### 1. Start Jaeger + +Jaeger has many configuration options, but is very easy to start in an "all-in-one" mode which uses +memory for trace storage (and is therefore non-persistent). The main advantage of "all-in-one" mode +being ease of use. + +For more detailed configuration options, refer to the [Jaeger +documentation](https://www.jaegertracing.io/docs/1.9/getting-started/). + +#### Using Docker + +If you have Docker available, the easier approach to running the Jaeger all-in-one is through +Docker, using the following command: + +```console +$ docker run \ + --rm \ + -e COLLECTOR_ZIPKIN_HTTP_PORT=9411 \ + -p 5775:5775/udp \ + -p 6831:6831/udp \ + -p 6832:6832/udp \ + -p 5778:5778 \ + -p 16686:16686 \ + -p 14268:14268 \ + -p 9411:9411 \ + jaegertracing/all-in-one:latest +``` + +#### Using the Jaeger process + +Without Docker, the all-in-one process is still easy to setup. + +1. Download the [latest Jaeger release](https://github.com/jaegertracing/jaeger/releases) for your + platform. +1. Extract the archive and run the `bin/all-in-one` process. + +This should start the process with the default listening ports. + +### 2. Configure the `GITLAB_TRACING` environment variable + +Once you have Jaeger running, you'll need to configure the `GITLAB_TRACING` variable with the +appropriate configuration string. + +**TL;DR:** If you are running everything on the same host, use the following value: + +```console +$ export GITLAB_TRACING="opentracing://jaeger?http_endpoint=http%3A%2F%2Flocalhost%3A14268%2Fapi%2Ftraces&sampler=const&sampler_param=1" +``` + +This configuration string uses the Jaeger driver `opentracing://jaeger` with the following options: + +| Name | Value | Description | +|------|-------|-------------| +| `http_endpoint` | `http://localhost:14268/api/traces` | Configures Jaeger to send trace information to the HTTP endpoint running on `http://localhost:14268/`. Alternatively, the `upd_endpoint` can be used. | +| `sampler` | `const` | Configures Jaeger to use the constant sampler (either on or off). | +| `sampler_param` | `1` | Configures the `const` sampler to sample _all_ traces. Using `0` would sample _no_ traces. | + +**Other parameter values are also possible:** + +| Name | Example | Description | +|------|-------|-------------| +| `udp_endpoint` | `localhost:6831` | This is the default. Configures Jaeger to send trace information to the UDP listener on port `6831` using compact thrift protocol. Note that we've experienced some issues with the [Jaeger Client for Ruby](https://github.com/salemove/jaeger-client-ruby) when using this protocol. | +| `sampler` | `probabalistic` | Configures Jaeger to use a probabilistic random sampler. The rate of samples is configured by the `sampler_param` value. | +| `sampler_param` | `0.01` | Use a ratio of `0.01` to configure the `probabalistic` sampler to randomly sample _1%_ of traces. | + +NOTE: **Note:** +The same `GITLAB_TRACING` value should to be configured in the environment +variables for all GitLab processes, including Workhorse, Gitaly, Rails, and Sidekiq. + +### 3. Start the GitLab application + +Once the `GITLAB_TRACING` environment variable is exported to all GitLab services, start the +application. + +When `GITLAB_TRACING` is configured properly, the application will log this on startup: + +```console +13:41:53 gitlab-workhorse.1 | 2019/02/12 13:41:53 Tracing enabled +... +13:41:54 gitaly.1 | 2019/02/12 13:41:54 Tracing enabled +... +``` + +If `GITLAB_TRACING` is not configured correctly, this will also be logged: + +```console +13:43:45 gitaly.1 | 2019/02/12 13:43:45 skipping tracing configuration step: tracer: unable to load driver mytracer +``` + +By default, GitLab ships with the Jaeger tracer, but other tracers can be included at compile time. +Details of how this can be done are included in the [LabKit tracing +documentation](https://godoc.org/gitlab.com/gitlab-org/labkit/tracing). + +If no log messages about tracing are emitted, the `GITLAB_TRACING` environment variable is likely +not set. + +### 4. Open the Jaeger Search UI + +By default, the Jaeger search UI is available at <http://localhost:16686/search>. + +TIP: **Tip:** +Don't forget that you will need to generate traces by using the application before +they appear in the Jaeger UI. + diff --git a/doc/development/documentation/index.md b/doc/development/documentation/index.md index 652fe3ea711..a4da34a50ce 100644 --- a/doc/development/documentation/index.md +++ b/doc/development/documentation/index.md @@ -511,10 +511,10 @@ Currently, the following tests are in place: 1. `docs lint`: Check that all internal (relative) links work correctly and that all cURL examples in API docs use the full switches. It's recommended - to [check locally](#previewing-locally) before pushing to GitLab by executing the command + to [check locally](#previewing-the-changes-live) before pushing to GitLab by executing the command `bundle exec nanoc check internal_links` on your local [`gitlab-docs`](https://gitlab.com/gitlab-com/gitlab-docs) directory. -1. [`ee_compat_check`](../automatic_ce_ee_merge.md#avoiding-ce-gt-ee-merge-conflicts-beforehand) (runs on CE only): +1. [`ee_compat_check`](../automatic_ce_ee_merge.md#avoiding-ce-ee-merge-conflicts-beforehand) (runs on CE only): When you submit a merge request to GitLab Community Edition (CE), there is this additional job that runs against Enterprise Edition (EE) and checks if your changes can apply cleanly to the EE codebase. diff --git a/doc/development/documentation/site_architecture/global_nav.md b/doc/development/documentation/site_architecture/global_nav.md index 0aa3c41a225..f2f4f5f0e1c 100644 --- a/doc/development/documentation/site_architecture/global_nav.md +++ b/doc/development/documentation/site_architecture/global_nav.md @@ -62,7 +62,7 @@ the consent of one of the technical writers. The global nav is built from two files: - [Data](#data-file) -- [Layout](#layout-file) +- [Layout](#layout-file-logic) The data file feeds the layout with the links to the docs. The layout organizes the data among the nav in containers properly [styled](#css-classes). diff --git a/doc/development/documentation/styleguide.md b/doc/development/documentation/styleguide.md index 7a3a8f25c2d..0c51d3832aa 100644 --- a/doc/development/documentation/styleguide.md +++ b/doc/development/documentation/styleguide.md @@ -36,7 +36,7 @@ gem will support all [GFM markup](../../user/markdown.md) in the future. For now use regular markdown markup, following the rules on this style guide. For a complete Kramdown reference, check the [GitLab Markdown Kramdown Guide](https://about.gitlab.com/handbook/product/technical-writing/markdown-guide/). Use Kramdown markup wisely: do not overuse its specific markup (e.g., `{:.class}`) as it will not render properly in -[`/help`](#gitlab-help). +[`/help`](index.md#gitlab-help). ## Content @@ -236,6 +236,24 @@ For other punctuation rules, please refer to the E.g., instead of writing something like `Read more about GitLab Issue Boards [here](LINK)`, write `Read more about [GitLab Issue Boards](LINK)`. +### Links to confidential issues + +Don't link directly to [confidential issues](../../user/project/issues/confidential_issues.md). These will fail for: + +- Those without sufficient permissions. +- Automated link checkers. + +Instead: + +- Mention in the text that the information is contained in a confidential issue. This will reduce confusion. +- Provide a link in back ticks (`` ` ``) so that those with access to the issue can easily navigate to it. + +Example: + +```md +For more information, see the [confidential issue](https://docs.gitlab.com/ee/user/project/issues/confidential_issues.html) `https://gitlab.com/gitlab-org/gitlab-ce/issues/<issue_number>`. +``` + ### Unlinking emails By default, all email addresses will render in an email tag on docs.gitlab.com. @@ -612,7 +630,7 @@ In this case: - The code blocks are indented one or more spaces under the list item to render correctly. - Different highlighting languages are used for each config in the code block. -- The [references](#references) guide is used for reconfigure/restart. +- The [GitLab Restart](#gitlab-restart) section is used to explain a required restart/reconfigure of GitLab. ## API diff --git a/doc/development/ee_features.md b/doc/development/ee_features.md index 3e85c0e1995..41a64044c68 100644 --- a/doc/development/ee_features.md +++ b/doc/development/ee_features.md @@ -161,7 +161,7 @@ still having access the class's implementation with `super`. There are a few gotchas with it: -- you should always [`extend ::Gitlab::Utils::Override`] and use `override` to +- you should always [`extend ::Gitlab::Utils::Override`](utilities.md#overridehttpsgitlabcomgitlab-orggitlab-ceblobmasterlibgitlabutilsoverriderb) and use `override` to guard the "overrider" method to ensure that if the method gets renamed in CE, the EE override won't be silently forgotten. - when the "overrider" would add a line in the middle of the CE @@ -273,8 +273,6 @@ module EE end ``` -[`extend ::Gitlab::Utils::Override`]: utilities.md#override - ##### Overriding CE class methods The same applies to class methods, except we want to use @@ -880,12 +878,104 @@ import bundle from 'ee_else_ce/protected_branches/protected_branches_bundle.js'; See the frontend guide [performance section](./fe_guide/performance.md) for information on managing page-specific javascript within EE. + +## Vue code in `assets/javascript` +### script tag + +#### Child Component only used in EE +To seperate Vue template differences we should [async import the components](https://vuejs.org/v2/guide/components-dynamic-async.html#Async-Components). + +Doing this allows for us to load the correct component in EE whilst in CE +we can load a empty component that renders nothing. This code **should** +exist in the CE repository as well as the EE repository. + +```html +<script> +export default { + components: { + EEComponent: () => import('ee_component/components/test.vue'), + }, +}; +</script> + +<template> + <div> + <ee-component /> + </div> +</template> +``` + +#### For JS code that is EE only, like props, computed properties, methods, etc, we will keep the current approach + - Since we [can't async load a mixin](https://github.com/vuejs/vue-loader/issues/418#issuecomment-254032223) we will use the [`ee_else_ce`](https://docs.gitlab.com/ee/development/ee_features.html#javascript-code-in-assetsjavascripts) alias we already have for webpack. + - This means all the EE specific props, computed properties, methods, etc that are EE only should be in a mixin in the `ee/` folder and we need to create a CE counterpart of the mixin + + ##### Example: + ```javascript + import mixin from 'ee_else_ce/path/mixin'; + + { + mixins: [mixin] + } + ``` +- Computed Properties/methods and getters only used in the child import still need a counterpart in CE + +- For store modules, we will need a CE counterpart too. +- You can see an MR with an example [here](https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/9762) + +#### `template` tag +* **EE Child components** + - Since we are using the async loading to check which component to load, we'd still use the component's name, check [this example](#child-component-only-used-in-ee). + +* **EE extra HTML** + - For the templates that have extra HTML in EE we will use the `ifEE` mixin with the `v-if` directive. + - You can either use the `template` tag as a wrapper or directly in the element, if there is only one element to be rendered in EE: + +```html + <template v-if="ifEE"> + <p>Several</p> + <p>non wrapper</p> + <p>elements</p> + <p>that are rendered</p> + <p>in EE only</p> + </template> +``` + + +```html + <ul v-if="renderIfEE"> + <li>One wrapped</li> + <li>element</li> + <li>that is rendered</li> + <li>in EE only</li> + </template> +``` + +### Non Vue Files +For regular JS files, the approach is similar. + +1. We will keep using the [`ee_else_ce`](https://docs.gitlab.com/ee/development/ee_features.html#javascript-code-in-assetsjavascripts) helper, this means that EE only code should be inside the `ee/` folder. + 1. An EE file should be created with the EE only code, and it should extend the CE counterpart. +1. For code inside functions that can't be extended, we will use an `if` statement with the `ifEE` helper + +##### Example: + +```javascript +import { ifEE } from '~/lib/utils/common_utils' +if (renderIfEE) { + $('.js-import-git-toggle-button').on('click', () => { + const $projectMirror = $('#project_mirror'); + + $projectMirror.attr('disabled', !$projectMirror.attr('disabled')); + }); +} +``` + ## SCSS code in `assets/stylesheets` To separate EE-specific styles in SCSS files, if a component you're adding styles for is limited to only EE, it is better to have a separate SCSS file in appropriate directory within `app/assets/stylesheets`. -See [backporting changes](#backporting-changes) for instructions on how to merge changes safely. +See [backporting changes](#backporting-changes-from-EE-to-CE) for instructions on how to merge changes safely. In some cases, this is not entirely possible or creating dedicated SCSS file is an overkill, e.g. a text style of some component is different for EE. In such cases, diff --git a/doc/development/fe_guide/architecture.md b/doc/development/fe_guide/architecture.md index aebb22caa15..c67389b169e 100644 --- a/doc/development/fe_guide/architecture.md +++ b/doc/development/fe_guide/architecture.md @@ -11,12 +11,9 @@ Architectural decisions should be accessible to everyone, so please document them in the relevant Merge Request discussion or by updating our documentation when appropriate. -You can find the Frontend Architecture experts on the [team page][team-page]. +You can find the Frontend Architecture experts on the [team page](https://about.gitlab.com/team). ## Examples You can find documentation about the desired architecture for a new feature -built with Vue.js [here][vue-section]. - -[team-page]: https://about.gitlab.com/team -[vue-section]: vue.md#frontend.html#how-to-build-a-new-feature-with-vue-js +built with Vue.js [here](vue.md). diff --git a/doc/development/fe_guide/vue.md b/doc/development/fe_guide/vue.md index 3cd70bd63fa..435fdf39fb4 100644 --- a/doc/development/fe_guide/vue.md +++ b/doc/development/fe_guide/vue.md @@ -122,7 +122,7 @@ Check this [page](vuex.md) for more details. ## Style guide -Please refer to the Vue section of our [style guide](style_guide_js.md#vue-js) +Please refer to the Vue section of our [style guide](style_guide_js.md#vuejs) for best practices while writing your Vue components and templates. ## Testing Vue Components @@ -132,7 +132,7 @@ Each Vue component has a unique output. This output is always present in the ren Although we can test each method of a Vue component individually, our goal must be to test the output of the render/template function, which represents the state at all times. -Make use of the [axios mock adapter](axios.md#mock-axios-response-on-tests) to mock data returned. +Make use of the [axios mock adapter](axios.md#mock-axios-response-in-tests) to mock data returned. Here's how we would test the Todo App above: diff --git a/doc/development/feature_flags.md b/doc/development/feature_flags.md index 67356a12eba..3271f9a7fb3 100644 --- a/doc/development/feature_flags.md +++ b/doc/development/feature_flags.md @@ -26,8 +26,9 @@ the time, you should execute `/chatops run feature set my_feature_flag 50`. ## Feature flags for user applications -GitLab does not yet support the use of feature flags in deployed user applications. -You can follow the progress on that [in the issue on our issue tracker](https://gitlab.com/gitlab-org/gitlab-ee/issues/779). +This document only covers feature flags used in the development of GitLab +itself. Feature flags in deployed user applications can be found at +[Feature Flags](https://docs.gitlab.com/ee/user/project/operations/feature_flags.html) ## Developing with feature flags diff --git a/doc/development/gitaly.md b/doc/development/gitaly.md index d5fc403bf8b..b1785e6f803 100644 --- a/doc/development/gitaly.md +++ b/doc/development/gitaly.md @@ -5,8 +5,9 @@ Workhorse and GitLab-Shell. ## Developing new Git features -Starting with GitLab 10.8, all new Git features should be developed in -Gitaly. +To read or write Git data, a request has to be made to Gitaly. This means that +if you're developing a new feature where you need data that's not yet available +in `lib/gitlab/git` changes have to be made to Gitaly. > This is a new process that is not clearly defined yet. If you want to contribute a Git feature and you're getting stuck, reach out to the @@ -56,6 +57,44 @@ If your test-suite is failing with Gitaly issues, as a first step, try running: rm -rf tmp/tests/gitaly ``` +## Legacy Rugged code + +While Gitaly can handle all Git access, many of GitLab customers still +run Gitaly atop NFS. The legacy Rugged implementation for Git calls may +be faster than the Gitaly RPC due to N+1 Gitaly calls and other +reasons. See [the +issue](https://gitlab.com/gitlab-org/gitlab-ce/issues/57317) for more +details. + +Until GitLab has eliminated most of these inefficiencies or the use of +NFS is discontinued for Git data, Rugged implementations of some of the +most commonly-used RPCs can be enabled via feature flags: + +* `rugged_find_commit` +* `rugged_get_tree_entries` +* `rugged_tree_entry` +* `rugged_commit_is_ancestor` + +A convenience Rake task can be used to enable or disable these flags +all together. To enable: + +```sh +bundle exec rake gitlab:features:enable_rugged +``` + +To disable: + +```sh +bundle exec rake gitlab:features:disable_rugged +``` + +Most of this code exists in the `lib/gitlab/git/rugged_impl` directory. + +NOTE: **Note:** You should NOT need to add or modify code related to +Rugged unless explicitly discussed with the [Gitaly +Team](https://gitlab.com/groups/gl-gitaly/group_members). This code will +NOT work on GitLab.com or other GitLab instances that do not use NFS. + ## `TooManyInvocationsError` errors During development and testing, you may experience `Gitlab::GitalyClient::TooManyInvocationsError` failures. diff --git a/doc/development/i18n/merging_translations.md b/doc/development/i18n/merging_translations.md index 2fa7558d30b..85284d8c714 100644 --- a/doc/development/i18n/merging_translations.md +++ b/doc/development/i18n/merging_translations.md @@ -32,8 +32,7 @@ clicking `Pause sync` on the [Crowdin integration settings page](https://translate.gitlab.com/project/gitlab-ee/settings#integration). When all failures are resolved, the translations need to be double -checked once more [as discussed in this -issue](https://gitlab.com/gitlab-org/gitlab-ce/issues/37850). +checked once more as discussed in [confidential issue](https://docs.gitlab.com/ee/user/project/issues/confidential_issues.html) `https://gitlab.com/gitlab-org/gitlab-ce/issues/37850`. ## Merging translations diff --git a/doc/development/img/distributed_tracing_jaeger_ui.png b/doc/development/img/distributed_tracing_jaeger_ui.png Binary files differnew file mode 100644 index 00000000000..57517dacced --- /dev/null +++ b/doc/development/img/distributed_tracing_jaeger_ui.png diff --git a/doc/development/img/distributed_tracing_performance_bar.png b/doc/development/img/distributed_tracing_performance_bar.png Binary files differnew file mode 100644 index 00000000000..c9998cedd2d --- /dev/null +++ b/doc/development/img/distributed_tracing_performance_bar.png diff --git a/doc/development/performance.md b/doc/development/performance.md index 2a287611bdf..0e21d45f57c 100644 --- a/doc/development/performance.md +++ b/doc/development/performance.md @@ -38,6 +38,7 @@ GitLab provides built-in tools to help improve performance and availability: - [Profiling](profiling.md). - [Sherlock](profiling.md#sherlock). +- [Distributed Tracing](distributed_tracing.md) - [GitLab Performance Monitoring](../administration/monitoring/performance/index.md). - [Request Profiling](../administration/monitoring/performance/request_profiling.md). - [QueryRecoder](query_recorder.md) for preventing `N+1` regressions. diff --git a/doc/development/profiling.md b/doc/development/profiling.md index f41d635de43..b2f3a105b23 100644 --- a/doc/development/profiling.md +++ b/doc/development/profiling.md @@ -10,6 +10,10 @@ There is a `Gitlab::Profiler.profile` method, and corresponding `bin/profile-url` script, that enable profiling a GET or POST request to a specific URL, either as an anonymous user (the default) or as a specific user. +NOTE: **Note:** The first argument to the profiler is either a full URL +(including the instance hostname) or an absolute path, including the +leading slash. + When using the script, command-line documentation is available by passing no arguments. @@ -86,10 +90,8 @@ that builds on this to add some additional niceties, such as allowing configuration with a single Yaml file for multiple URLs, and uploading of the profile and log output to S3. -For GitLab.com, currently the latest profiling data has been [moved from -Redash to Looker](https://gitlab.com/gitlab-com/Product/issues/5#note_121194467). -We are [currently investigating how to make this data -public](https://gitlab.com/meltano/looker/issues/294). +For GitLab.com, you can find the latest results here: +<http://redash.gitlab.com/dashboard/gitlab-profiler-statistics> ## Sherlock diff --git a/doc/development/rolling_out_changes_using_feature_flags.md b/doc/development/rolling_out_changes_using_feature_flags.md index ef1aba95712..4e0d1b74bc9 100644 --- a/doc/development/rolling_out_changes_using_feature_flags.md +++ b/doc/development/rolling_out_changes_using_feature_flags.md @@ -135,6 +135,20 @@ want to wait several hours or even days. This is entirely up to you, just make sure it is clearly communicated to your team, and the Production team if you anticipate any potential problems. +Feature gates can also be actor based, for example a feature could first be +enabled for only the `gitlab-ce` project. The project is passed by supplying a +`--project` flag: + +``` +/chatops run feature set --project=gitlab-org/gitlab-ce some_feature true +``` + +For groups the `--group` flag is available: + +``` +/chatops run feature set --group=gitlab-org some_feature true +``` + Once a change is deemed stable, submit a new merge request to remove the feature flag. This ensures the change is available to all users and self-hosted instances. Make sure to add the ~"feature flag" label to this merge request so @@ -188,3 +202,12 @@ to be shipped. You can do this via ChatOps: Note that you can do this at any time, even before the merge request using the flag has been merged! + +### Cleaning up + +When a feature gate has been removed from the code base, the value still exists +in the database. This can be removed through ChatOps: + +``` +/chatops run feature delete some_feature +``` diff --git a/doc/development/testing_guide/best_practices.md b/doc/development/testing_guide/best_practices.md index 2c8d488877b..cfe0e6f70fc 100644 --- a/doc/development/testing_guide/best_practices.md +++ b/doc/development/testing_guide/best_practices.md @@ -40,7 +40,7 @@ bundle exec rspec spec/[path]/[to]/[spec].rb to separate phases. - Use `Gitlab.config.gitlab.host` rather than hard coding `'localhost'` - Don't assert against the absolute value of a sequence-generated attribute (see - [Gotchas](../gotchas.md#dont-assert-against-the-absolute-value-of-a-sequence-generated-attribute)). + [Gotchas](../gotchas.md#do-not-assert-against-the-absolute-value-of-a-sequence-generated-attribute)). - Don't supply the `:each` argument to hooks since it's the default. - On `before` and `after` hooks, prefer it scoped to `:context` over `:all` - When using `evaluate_script("$('.js-foo').testSomething()")` (or `execute_script`) which acts on a given element, @@ -168,12 +168,13 @@ instead of 30+ seconds in case of a regular `spec_helper`. ### `let` variables -GitLab's RSpec suite has made extensive use of `let` variables to reduce -duplication. However, this sometimes [comes at the cost of clarity][lets-not], +GitLab's RSpec suite has made extensive use of `let`(along with it strict, non-lazy +version `let!`) variables to reduce duplication. However, this sometimes [comes at the cost of clarity][lets-not], so we need to set some guidelines for their use going forward: -- `let` variables are preferable to instance variables. Local variables are - preferable to `let` variables. +- `let!` variables are preferable to instance variables. `let` variables + are preferable to `let!` variables. Local variables are preferable to + `let` variables. - Use `let` to reduce duplication throughout an entire spec file. - Don't use `let` to define variables used by a single test; define them as local variables inside the test's `it` block. @@ -183,6 +184,9 @@ so we need to set some guidelines for their use going forward: - Try to avoid overriding the definition of one `let` variable with another. - Don't define a `let` variable that's only used by the definition of another. Use a helper method instead. +- `let!` variables should be used only in case if strict evaluation with defined + order is required, otherwise `let` will suffice. Remember that `let` is lazy and won't + be evaluated until it is referenced. [lets-not]: https://robots.thoughtbot.com/lets-not diff --git a/doc/development/testing_guide/ci.md b/doc/development/testing_guide/ci.md index 5aa668290b4..7a7fca46534 100644 --- a/doc/development/testing_guide/ci.md +++ b/doc/development/testing_guide/ci.md @@ -31,7 +31,11 @@ After that, the next pipeline will use the up-to-date The GitLab test suite is [monitored] for the `master` branch, and any branch that includes `rspec-profile` in their name. +A [public dashboard] is available for everyone to see. Feel free to look at the +slowest test files and try to improve them. + [monitored]: ../performance.md#rspec-profiling +[public dashboard]: https://redash.gitlab.com/public/dashboards/l1WhHXaxrCWM5Ai9D7YDqHKehq6OU3bx5gssaiWe?org_slug=default ## CI setup diff --git a/doc/development/testing_guide/frontend_testing.md b/doc/development/testing_guide/frontend_testing.md index 0470a071d39..5b66e513a76 100644 --- a/doc/development/testing_guide/frontend_testing.md +++ b/doc/development/testing_guide/frontend_testing.md @@ -13,6 +13,42 @@ in the future. See the [Testing Standards and Style Guidelines](index.md) page for more information on general testing practices at GitLab. +## Jest + +GitLab has started to migrate tests to the (Jest)[https://jestjs.io] +testing framework. You can read a [detailed evaluation](https://gitlab.com/gitlab-org/gitlab-ce/issues/49171) +of Jest compared to our use of Karma and Jasmine. In summary, it will allow us +to improve the performance and consistency of our frontend tests. + +Jest tests can be found in `/spec/frontend` and `/ee/spec/frontend` in EE. + +It is not yet a requirement to use Jest. You can view the +[epic](https://gitlab.com/groups/gitlab-org/-/epics/873) of issues +we need to solve before being able to use Jest for all our needs. + +### Timeout error + +The default timeout for Jest is set in +[`/spec/frontend/test_setup.js`](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/spec/frontend/test_setup.js). + +If your test exceeds that time, it will fail. + +If you cannot improve the performance of the tests, you can increase the timeout +for a specific test using +[`jest.setTimeout`](https://jestjs.io/docs/en/jest-object#jestsettimeouttimeout). + +```javascript +beforeAll(() => { + jest.setTimeout(500); +}); + +describe('Component', () => { + // ... +}); +``` + +Remember that the performance of each test depends on the environment. + ## Karma test suite GitLab uses the [Karma][karma] test runner with [Jasmine] as its test @@ -134,7 +170,7 @@ placeholders, and recalling when they are called and the arguments that are passed to them. These tools should be used liberally, to test for expected behavior, to mock responses, and to block unwanted side effects (such as a method that would generate a network request or alter `window.location`). The -documentation for these methods can be found in the [jasmine introduction page](https://jasmine.github.io/2.0/introduction.html#section-Spies). +documentation for these methods can be found in the [Jasmine introduction page](https://jasmine.github.io/2.0/introduction.html#section-Spies). Sometimes you may need to spy on a method that is directly imported by another module. GitLab has a custom `spyOnDependency` method which utilizes @@ -168,7 +204,7 @@ export of a module who's import you want to stub, rather than an object which contains a method you wish to stub (if the module does not have a default export, one is be generated by the babel plugin). The second parameter is the name of the import you wish to change. The result of the function is a Spy -object which can be treated like any other jasmine spy object. +object which can be treated like any other Jasmine spy object. Further documentation on the babel rewire pluign API can be found on [its repository Readme doc](https://github.com/speedskater/babel-plugin-rewire#babel-plugin-rewire). @@ -177,6 +213,14 @@ Further documentation on the babel rewire pluign API can be found on If you cannot avoid using [`setTimeout`](https://developer.mozilla.org/en-US/docs/Web/API/WindowOrWorkerGlobalScope/setTimeout) in tests, please use the [Jasmine mock clock](https://jasmine.github.io/api/2.9/Clock.html). +#### Migrating flaky Karma tests to Jest + +Some of our Karma tests are flaky because they access the properties of a shared scope. +This also means that they are not easily parallelized. + +Migrating flaky Karma tests to Jest will help significantly as each test is executed +in an isolated scope, improving performance and predictability. + ### Vue.js unit tests See this [section][vue-test]. @@ -194,21 +238,21 @@ is sufficient (and saves you some time). ### Live testing and focused testing -While developing locally, it may be helpful to keep karma running so that you +While developing locally, it may be helpful to keep Karma running so that you can get instant feedback on as you write tests and modify code. To do this -you can start karma with `yarn run karma-start`. It will compile the javascript +you can start Karma with `yarn run karma-start`. It will compile the javascript assets and run a server at `http://localhost:9876/` where it will automatically run the tests on any browser which connects to it. You can enter that url on multiple browsers at once to have it run the tests on each in parallel. -While karma is running, any changes you make will instantly trigger a recompile +While Karma is running, any changes you make will instantly trigger a recompile and retest of the entire test suite, so you can see instantly if you've broken -a test with your changes. You can use [jasmine focused][jasmine-focus] or -excluded tests (with `fdescribe` or `xdescribe`) to get karma to run only the +a test with your changes. You can use [Jasmine focused][jasmine-focus] or +excluded tests (with `fdescribe` or `xdescribe`) to get Karma to run only the tests you want while you're working on a specific feature, but make sure to remove these directives when you commit your code. -It is also possible to only run karma on specific folders or files by filtering +It is also possible to only run Karma on specific folders or files by filtering the run tests via the argument `--filter-spec` or short `-f`: ```bash diff --git a/doc/development/testing_guide/review_apps.md b/doc/development/testing_guide/review_apps.md index 703e342fc13..fda3ff57316 100644 --- a/doc/development/testing_guide/review_apps.md +++ b/doc/development/testing_guide/review_apps.md @@ -95,6 +95,14 @@ You can also manually start the `review-qa-all`: it runs the full QA suite. Note that both jobs first wait for the `review-deploy` job to be finished. +## Performance Metrics + +On every [pipeline][gitlab-pipeline] during the `test` stage, the +`review-performance` job is automatically started: this job does basic +browser performance testing using [Sitespeed.io Container](https://docs.gitlab.com/ee/user/project/merge_requests/browser_performance_testing.html) . + +This job waits for the `review-deploy` job to be finished. + ## How to? ### Log into my Review App? diff --git a/doc/development/testing_guide/testing_levels.md b/doc/development/testing_guide/testing_levels.md index a7a3459719b..5d46833a1e2 100644 --- a/doc/development/testing_guide/testing_levels.md +++ b/doc/development/testing_guide/testing_levels.md @@ -46,7 +46,7 @@ They're useful to test permissions, redirections, what view is rendered etc. | `app/mailers/` | `spec/mailers/` | RSpec | | | `lib/api/` | `spec/requests/api/` | RSpec | | | `lib/ci/api/` | `spec/requests/ci/api/` | RSpec | | -| `app/assets/javascripts/` | `spec/javascripts/` | Karma | More details in the [JavaScript](#javascript) section. | +| `app/assets/javascripts/` | `spec/javascripts/` | Karma | More details in the [Karma JavaScript test suite](frontend_testing.md#karma-test-suite) section. | ### About controller tests @@ -210,7 +210,7 @@ trade-off: - Integration tests are a bit more expensive, but don't abuse them. A system test is often better than an integration test that is stubbing a lot of internals. - System tests are expensive (compared to unit tests), even more if they require - a JavaScript driver. Make sure to follow the guidelines in the [Speed](#test-speed) + a JavaScript driver. Make sure to follow the guidelines in the [Speed](best_practices.md#test-speed) section. Another way to see it is to think about the "cost of tests", this is well |
