From 3eb814543ab39eb3cda77dce00eaab1414e944fc Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Tue, 22 Jun 2021 00:07:53 +0000 Subject: [PATCH] Add latest changes from gitlab-org/gitlab@master --- app/models/ci/runner.rb | 2 +- .../ci_runners_tokens_optional_encryption.yml | 8 - .../development/multiple_database_metrics.yml | 8 + .../external_pipeline_validation.md | 2 +- doc/ci/caching/index.md | 2 +- doc/ci/docker/index.md | 4 +- doc/ci/docker/using_docker_build.md | 2 +- doc/ci/docker/using_docker_images.md | 2 +- doc/ci/docker/using_kaniko.md | 2 +- doc/ci/enable_or_disable_ci.md | 2 +- .../laravel_with_gitlab_and_envoy/index.md | 2 +- doc/ci/examples/php.md | 2 +- doc/ci/examples/semantic-release.md | 2 +- doc/ci/git_submodules.md | 2 +- doc/ci/jobs/index.md | 2 +- doc/ci/merge_request_pipelines/index.md | 2 +- doc/ci/parent_child_pipelines.md | 2 +- doc/ci/ssh_keys/index.md | 2 +- doc/ci/troubleshooting.md | 2 +- doc/ci/yaml/gitlab_ci_yaml.md | 2 +- doc/ci/yaml/includes.md | 2 +- doc/ci/yaml/script.md | 2 +- doc/development/api_graphql_styleguide.md | 152 +++++++++++++++--- .../cicd_reference_documentation_guide.md | 2 +- doc/development/cicd/index.md | 2 +- lib/gitlab/database.rb | 10 ++ .../metrics/subscribers/active_record.rb | 17 ++ spec/lib/gitlab/database_spec.rb | 17 ++ .../metrics/subscribers/active_record_spec.rb | 2 +- .../sidekiq_logging/structured_logger_spec.rb | 5 +- ...ctive_record_subscriber_shared_examples.rb | 32 +++- 31 files changed, 233 insertions(+), 64 deletions(-) delete mode 100644 config/feature_flags/development/ci_runners_tokens_optional_encryption.yml create mode 100644 config/feature_flags/development/multiple_database_metrics.yml diff --git a/app/models/ci/runner.rb b/app/models/ci/runner.rb index 862cf00533c..a541dca47de 100644 --- a/app/models/ci/runner.rb +++ b/app/models/ci/runner.rb @@ -13,7 +13,7 @@ module Ci include Gitlab::Utils::StrongMemoize include TaggableQueries - add_authentication_token_field :token, encrypted: -> { Feature.enabled?(:ci_runners_tokens_optional_encryption, default_enabled: true) ? :optional : :required } + add_authentication_token_field :token, encrypted: :optional enum access_level: { not_protected: 0, diff --git a/config/feature_flags/development/ci_runners_tokens_optional_encryption.yml b/config/feature_flags/development/ci_runners_tokens_optional_encryption.yml deleted file mode 100644 index 147dc2d0ce9..00000000000 --- a/config/feature_flags/development/ci_runners_tokens_optional_encryption.yml +++ /dev/null @@ -1,8 +0,0 @@ ---- -name: ci_runners_tokens_optional_encryption -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/8638 -rollout_issue_url: -milestone: '11.6' -type: development -group: group::pipeline execution -default_enabled: true diff --git a/config/feature_flags/development/multiple_database_metrics.yml b/config/feature_flags/development/multiple_database_metrics.yml new file mode 100644 index 00000000000..7a700982022 --- /dev/null +++ b/config/feature_flags/development/multiple_database_metrics.yml @@ -0,0 +1,8 @@ +--- +name: multiple_database_metrics +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/63490 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/333227 +milestone: '14.1' +type: development +group: group::sharding +default_enabled: false diff --git a/doc/administration/external_pipeline_validation.md b/doc/administration/external_pipeline_validation.md index 9fc65fdd0b5..738cf591210 100644 --- a/doc/administration/external_pipeline_validation.md +++ b/doc/administration/external_pipeline_validation.md @@ -5,7 +5,7 @@ info: To determine the technical writer assigned to the Stage/Group associated w type: reference, howto --- -# External pipeline validation +# External pipeline validation **(FREE SELF)** You can use an external service to validate a pipeline before it's created. diff --git a/doc/ci/caching/index.md b/doc/ci/caching/index.md index 136c6c282df..856caf26794 100644 --- a/doc/ci/caching/index.md +++ b/doc/ci/caching/index.md @@ -5,7 +5,7 @@ info: To determine the technical writer assigned to the Stage/Group associated w type: index, concepts, howto --- -# Caching in GitLab CI/CD +# Caching in GitLab CI/CD **(FREE)** A cache is one or more files that a job downloads and saves. Subsequent jobs that use the same cache don't have to download the files again, so they execute more quickly. diff --git a/doc/ci/docker/index.md b/doc/ci/docker/index.md index 20599c5ca85..4f6da72af12 100644 --- a/doc/ci/docker/index.md +++ b/doc/ci/docker/index.md @@ -6,9 +6,9 @@ comments: false type: index --- -# Docker integration +# Docker integration **(FREE)** -There are two primary ways to incorporate [Docker](https://www.docker.com) in your CI/CD workflow. +There are two primary ways to incorporate [Docker](https://www.docker.com) into your CI/CD workflow: - **[Run your CI/CD jobs](using_docker_images.md) in Docker containers.** diff --git a/doc/ci/docker/using_docker_build.md b/doc/ci/docker/using_docker_build.md index 9dac08324c8..94af30bd73f 100644 --- a/doc/ci/docker/using_docker_build.md +++ b/doc/ci/docker/using_docker_build.md @@ -5,7 +5,7 @@ info: To determine the technical writer assigned to the Stage/Group associated w type: concepts, howto --- -# Use Docker to build Docker images +# Use Docker to build Docker images **(FREE)** You can use GitLab CI/CD with Docker to create Docker images. For example, you can create a Docker image of your application, diff --git a/doc/ci/docker/using_docker_images.md b/doc/ci/docker/using_docker_images.md index 29f4053f720..d6e4bc5e695 100644 --- a/doc/ci/docker/using_docker_images.md +++ b/doc/ci/docker/using_docker_images.md @@ -5,7 +5,7 @@ info: To determine the technical writer assigned to the Stage/Group associated w type: concepts, howto --- -# Run your CI/CD jobs in Docker containers +# Run your CI/CD jobs in Docker containers **(FREE)** You can run your CI/CD jobs in separate, isolated Docker containers. diff --git a/doc/ci/docker/using_kaniko.md b/doc/ci/docker/using_kaniko.md index 05769cc8f75..5e294d14f04 100644 --- a/doc/ci/docker/using_kaniko.md +++ b/doc/ci/docker/using_kaniko.md @@ -5,7 +5,7 @@ info: To determine the technical writer assigned to the Stage/Group associated w type: howto --- -# Use kaniko to build Docker images +# Use kaniko to build Docker images **(FREE)** > [Introduced](https://gitlab.com/gitlab-org/gitlab-foss/-/issues/45512) in GitLab 11.2. Requires GitLab Runner 11.2 and above. diff --git a/doc/ci/enable_or_disable_ci.md b/doc/ci/enable_or_disable_ci.md index 4633cc1a3f8..268be535591 100644 --- a/doc/ci/enable_or_disable_ci.md +++ b/doc/ci/enable_or_disable_ci.md @@ -5,7 +5,7 @@ info: To determine the technical writer assigned to the Stage/Group associated w type: howto --- -# How to enable or disable GitLab CI/CD +# How to enable or disable GitLab CI/CD **(FREE)** To effectively use GitLab CI/CD, you need: diff --git a/doc/ci/examples/laravel_with_gitlab_and_envoy/index.md b/doc/ci/examples/laravel_with_gitlab_and_envoy/index.md index 5f08f2954f5..7080ce82b2c 100644 --- a/doc/ci/examples/laravel_with_gitlab_and_envoy/index.md +++ b/doc/ci/examples/laravel_with_gitlab_and_envoy/index.md @@ -11,7 +11,7 @@ date: 2017-08-31 -# Test and deploy Laravel applications with GitLab CI/CD and Envoy +# Test and deploy Laravel applications with GitLab CI/CD and Envoy **(FREE)** ## Introduction diff --git a/doc/ci/examples/php.md b/doc/ci/examples/php.md index fc639b19ca0..9e0ff992d40 100644 --- a/doc/ci/examples/php.md +++ b/doc/ci/examples/php.md @@ -5,7 +5,7 @@ info: To determine the technical writer assigned to the Stage/Group associated w type: tutorial --- -# Testing PHP projects +# Testing PHP projects **(FREE)** This guide covers basic building instructions for PHP projects. diff --git a/doc/ci/examples/semantic-release.md b/doc/ci/examples/semantic-release.md index 28a0080626a..e7b9b9b51ad 100644 --- a/doc/ci/examples/semantic-release.md +++ b/doc/ci/examples/semantic-release.md @@ -4,7 +4,7 @@ group: Package info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments --- -# Publish npm packages to the GitLab Package Registry using semantic-release +# Publish npm packages to the GitLab Package Registry using semantic-release **(FREE)** This guide demonstrates how to automatically publish npm packages to the [GitLab Package Registry](../../user/packages/npm_registry/index.md) by using [semantic-release](https://github.com/semantic-release/semantic-release). diff --git a/doc/ci/git_submodules.md b/doc/ci/git_submodules.md index f0ea5ed582c..0df653acca4 100644 --- a/doc/ci/git_submodules.md +++ b/doc/ci/git_submodules.md @@ -5,7 +5,7 @@ info: To determine the technical writer assigned to the Stage/Group associated w type: reference --- -# Using Git submodules with GitLab CI/CD +# Using Git submodules with GitLab CI/CD **(FREE)** Use [Git submodules](https://git-scm.com/book/en/v2/Git-Tools-Submodules) to keep a Git repository as a subdirectory of another Git repository. You can clone another diff --git a/doc/ci/jobs/index.md b/doc/ci/jobs/index.md index 7a57d8abf0d..f359eecf01b 100644 --- a/doc/ci/jobs/index.md +++ b/doc/ci/jobs/index.md @@ -4,7 +4,7 @@ group: Pipeline Execution info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments --- -# Jobs +# Jobs **(FREE)** Pipeline configuration begins with jobs. Jobs are the most fundamental element of a `.gitlab-ci.yml` file. diff --git a/doc/ci/merge_request_pipelines/index.md b/doc/ci/merge_request_pipelines/index.md index c706d346146..a9a429292da 100644 --- a/doc/ci/merge_request_pipelines/index.md +++ b/doc/ci/merge_request_pipelines/index.md @@ -6,7 +6,7 @@ type: reference, index last_update: 2019-07-03 --- -# Pipelines for merge requests +# Pipelines for merge requests **(FREE)** > [Introduced](https://gitlab.com/gitlab-org/gitlab-foss/-/issues/15310) in GitLab 11.6. diff --git a/doc/ci/parent_child_pipelines.md b/doc/ci/parent_child_pipelines.md index 82bac7c51d2..07ad5b5401e 100644 --- a/doc/ci/parent_child_pipelines.md +++ b/doc/ci/parent_child_pipelines.md @@ -5,7 +5,7 @@ info: To determine the technical writer assigned to the Stage/Group associated w type: reference --- -# Parent-child pipelines +# Parent-child pipelines **(FREE)** > [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/16094) in GitLab 12.7. diff --git a/doc/ci/ssh_keys/index.md b/doc/ci/ssh_keys/index.md index 2222ed0aa43..85755f9a179 100644 --- a/doc/ci/ssh_keys/index.md +++ b/doc/ci/ssh_keys/index.md @@ -5,7 +5,7 @@ info: To determine the technical writer assigned to the Stage/Group associated w type: tutorial --- -# Using SSH keys with GitLab CI/CD +# Using SSH keys with GitLab CI/CD **(FREE)** GitLab currently doesn't have built-in support for managing SSH keys in a build environment (where the GitLab Runner runs). diff --git a/doc/ci/troubleshooting.md b/doc/ci/troubleshooting.md index 24a37900e6a..bbbda7815d0 100644 --- a/doc/ci/troubleshooting.md +++ b/doc/ci/troubleshooting.md @@ -5,7 +5,7 @@ info: To determine the technical writer assigned to the Stage/Group associated w type: reference --- -# Troubleshooting CI/CD +# Troubleshooting CI/CD **(FREE)** GitLab provides several tools to help make troubleshooting your pipelines easier. diff --git a/doc/ci/yaml/gitlab_ci_yaml.md b/doc/ci/yaml/gitlab_ci_yaml.md index 266b35bd27f..72e3f644d26 100644 --- a/doc/ci/yaml/gitlab_ci_yaml.md +++ b/doc/ci/yaml/gitlab_ci_yaml.md @@ -6,7 +6,7 @@ type: reference --- -# The .gitlab-ci.yml file +# The .gitlab-ci.yml file **(FREE)** diff --git a/doc/ci/yaml/includes.md b/doc/ci/yaml/includes.md index 549a6fb964b..385d0fbb80a 100644 --- a/doc/ci/yaml/includes.md +++ b/doc/ci/yaml/includes.md @@ -5,7 +5,7 @@ info: To determine the technical writer assigned to the Stage/Group associated w type: reference --- -# GitLab CI/CD include examples +# GitLab CI/CD include examples **(FREE)** In addition to the [`includes` examples](README.md#include) listed in the [GitLab CI YAML reference](README.md), this page lists more variations of `include` diff --git a/doc/ci/yaml/script.md b/doc/ci/yaml/script.md index 2e5517f6190..b695bc90b31 100644 --- a/doc/ci/yaml/script.md +++ b/doc/ci/yaml/script.md @@ -5,7 +5,7 @@ info: To determine the technical writer assigned to the Stage/Group associated w type: reference --- -# GitLab CI/CD script syntax +# GitLab CI/CD script syntax **(FREE)** You can use special syntax in [`script`](README.md#script) sections to: diff --git a/doc/development/api_graphql_styleguide.md b/doc/development/api_graphql_styleguide.md index 4d521d11a69..b3fbae47f82 100644 --- a/doc/development/api_graphql_styleguide.md +++ b/doc/development/api_graphql_styleguide.md @@ -22,7 +22,7 @@ which is exposed as an API endpoint at `/api/graphql`. ## Deep Dive In March 2019, Nick Thomas hosted a Deep Dive (GitLab team members only: `https://gitlab.com/gitlab-org/create-stage/issues/1`) -on the GitLab [GraphQL API](../api/graphql/index.md) to share his domain specific knowledge +on the GitLab [GraphQL API](../api/graphql/index.md) to share domain-specific knowledge with anyone who may work in this part of the codebase in the future. You can find the [recording on YouTube](https://www.youtube.com/watch?v=-9L_1MWrjkg), and the slides on @@ -83,7 +83,7 @@ developers must familiarize themselves with our [Deprecation and Removal process Breaking changes are: -- Removing or renaming a field, argument, enum value or mutation. +- Removing or renaming a field, argument, enum value, or mutation. - Changing the type of a field, argument or enum value. - Raising the [complexity](#max-complexity) of a field or complexity multipliers in a resolver. - Changing a field from being _not_ nullable (`null: false`) to nullable (`null: true`), as @@ -96,7 +96,7 @@ discussed in [Nullable fields](#nullable-fields). Fields that use the [`feature_flag` property](#feature_flag-property) and the flag is disabled by default are exempt from the deprecation process, and can be removed at any time without notice. -See the [deprecating fields and enum values](#deprecating-fields-arguments-and-enum-values) section for how to deprecate items. +See the [deprecating fields, arguments, and enum values](#deprecating-fields-arguments-and-enum-values) section for how to deprecate items. ## Global IDs @@ -110,6 +110,7 @@ See also: - [Exposing Global IDs](#exposing-global-ids). - [Mutation arguments](#object-identifier-arguments). +- [Deprecating Global IDs](#deprecate-global-ids). We have a custom scalar type (`Types::GlobalIDType`) which should be used as the type of input and output arguments when the value is a `GlobalID`. The benefits @@ -117,12 +118,12 @@ of using this type instead of `ID` are: - it validates that the value is a `GlobalID` - it parses it into a `GlobalID` before passing it to user code -- it can be parameterized on the type of the object (e.g. +- it can be parameterized on the type of the object (for example, `GlobalIDType[Project]`) which offers even better validation and security. Consider using this type for all new arguments and result types. Remember that it is perfectly possible to parameterize this type with a concern or a -supertype, if you want to accept a wider range of objects (e.g. +supertype, if you want to accept a wider range of objects (such as `GlobalIDType[Issuable]` vs `GlobalIDType[Issue]`). ## Types @@ -341,7 +342,7 @@ For example, instead of `latest_pipeline`, use `pipelines(last: 1)`. By default, the API returns at most a maximum number of records defined in [`app/graphql/gitlab_schema.rb`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/app/graphql/gitlab_schema.rb) -per page within a connection and this will also be the default number of records +per page in a connection and this is also the default number of records returned per page if no limiting arguments (`first:` or `last:`) are provided by a client. The `max_page_size` argument can be used to specify a different page size limit @@ -369,7 +370,7 @@ Complexity is described in [our client documentation](../api/graphql/index.md#ma Complexity limits are defined in [`app/graphql/gitlab_schema.rb`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/app/graphql/gitlab_schema.rb). -By default, fields will add `1` to a query's complexity score. This can be overridden by +By default, fields add `1` to a query's complexity score. This can be overridden by [providing a custom `complexity`](https://graphql-ruby.org/queries/complexity_and_depth.html) value for a field. Developers should specify higher complexity for fields that cause more _work_ to be performed @@ -390,7 +391,7 @@ field :blob, type: Types::Snippets::BlobType, calls_gitaly: true ``` -This will increment the [`complexity` score](#field-complexity) of the field by `1`. +This increments the [`complexity` score](#field-complexity) of the field by `1`. If a resolver calls Gitaly, it can be annotated with `BaseResolver.calls_gitaly!`. This passes `calls_gitaly: true` to any @@ -480,7 +481,7 @@ You can refer to these guidelines to decide which approach to use: The `feature_flag` property allows you to toggle the field's [visibility](https://graphql-ruby.org/authorization/visibility.html) -within the GraphQL schema. This removes the field from the schema +in the GraphQL schema. This removes the field from the schema when the flag is disabled. A description is [appended](https://gitlab.com/gitlab-org/gitlab/-/blob/497b556/app/graphql/types/base_field.rb#L44-53) @@ -595,6 +596,105 @@ end If the field, argument, or enum value being deprecated is not being replaced, a descriptive deprecation `reason` should be given. +### Deprecate Global IDs + +We use the [`rails/globalid`](https://github.com/rails/globalid) gem to generate and parse +Global IDs, so as such they are coupled to model names. When we rename a +model, its Global ID changes. + +If the Global ID is used as an _argument_ type anywhere in the schema, then the Global ID +change would normally constitute a breaking change. + +To continue to support clients using the old Global ID argument, we add a deprecation +to `Gitlab::GlobalId::Deprecations`. + +NOTE: +If the Global ID is _only_ [exposed as a field](#exposing-global-ids) then we do not need to +deprecate it. We consider the change to the way a Global ID is expressed in a field to be +backwards-compatible. We expect that clients don't parse these values: they are meant to +be treated as opaque tokens, and any structure in them is incidental and not to be relied on. + +**Example scenario:** + +This example scenario is based on this [merge request](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/62645). + +A model named `PrometheusService` is to be renamed `Integrations::Prometheus`. The old model +name is used to create a Global ID type that is used as an argument for a mutation: + +```ruby +# Mutations::UpdatePrometheus: + +argument :id, Types::GlobalIDType[::PrometheusService], + required: true, + description: "The ID of the integration to mutate." +``` + +Clients call the mutation by passing a Global ID string that looks like +`"gid://gitlab/PrometheusService/1"`, named as `PrometheusServiceID`, as the `input.id` argument: + +```graphql +mutation updatePrometheus($id: PrometheusServiceID!, $active: Boolean!) { + prometheusIntegrationUpdate(input: { id: $id, active: $active }) { + errors + integration { + active + } + } +} +``` + +We rename the model to `Integrations::Prometheus`, and then update the codebase with the new name. +When we come to update the mutation, we pass the renamed model to `Types::GlobalIDType[]`: + +```ruby +# Mutations::UpdatePrometheus: + +argument :id, Types::GlobalIDType[::Integrations::Prometheus], + required: true, + description: "The ID of the integration to mutate." +``` + +This would cause a breaking change to the mutation, as the API now rejects clients who +pass an `id` argument as `"gid://gitlab/PrometheusService/1"`, or that specify the argument +type as `PrometheusServiceID` in the query signature. + +To allow clients to continue to interact with the mutation unchanged, edit the `DEPRECATIONS` constant in +`Gitlab::GlobalId::Deprecations` and add a new `Deprecation` to the array: + +```ruby +DEPRECATIONS = [ + Deprecation.new(old_model_name: 'PrometheusService', new_model_name: 'Integrations::Prometheus', milestone: '14.0') +].freeze +``` + +Then follow our regular [deprecation process](../api/graphql/index.md#deprecation-and-removal-process). To later remove +support for the former argument style, remove the `Deprecation`: + +```ruby +DEPRECATIONS = [].freeze +``` + +During the deprecation period the API accepts these values for the argument: + +Formatted as either: + +- `"gid://gitlab/PrometheusService/1"` +- `"gid://gitlab/Integrations::Prometheus/1"` + +And query signatures recognizes the type under the old and new type names, accepting either: + +- `PrometheusServiceID` +- `IntegrationsPrometheusID` + +NOTE: +Although queries that use the old name (`PrometheusServiceID` in this example) would be +considered valid and executable by the API, validator tools would consider them invalid. +This is because we are deprecating using a bespoke method outside of the +[`@deprecated` directive](https://spec.graphql.org/June2018/#sec--deprecated), so validators are not +aware of the support. + +The documentation mentions that the old Global ID style is now deprecated. + See also [Aliasing and deprecating mutations](#aliasing-and-deprecating-mutations). ## Enums @@ -784,7 +884,7 @@ field :genus, see: { 'Wikipedia page on genera' => 'https://wikipedia.org/wiki/Genus' } ``` -This will render in our documentation as: +This renders in our documentation as: ```markdown A taxonomic genus. See: [Wikipedia page on genera](https://wikipedia.org/wiki/Genus) @@ -859,11 +959,11 @@ overhead. If you are writing: ### Error handling -Resolvers may raise errors, which will be converted to top-level errors as +Resolvers may raise errors, which are converted to top-level errors as appropriate. All anticipated errors should be caught and transformed to an appropriate GraphQL error (see [`Gitlab::Graphql::Errors`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/lib/gitlab/graphql/errors.rb)). -Any uncaught errors will be suppressed and the client will receive the message +Any uncaught errors are suppressed and the client receives the message `Internal service error`. The one special case is permission errors. In the REST API we return @@ -874,7 +974,7 @@ Query resolvers **should not raise errors for unauthorized resources**. The rationale for this is that clients must not be able to distinguish between the absence of a record and the presence of one they do not have access to. To -do so is a security vulnerability, since it leaks information we want to keep +do so is a security vulnerability, because it leaks information we want to keep hidden. In most cases you don't need to worry about this - this is handled correctly by @@ -1037,7 +1137,7 @@ class MyThingResolver < BaseResolver end ``` -By default, fields defined in `#preloads` will be preloaded if that field +By default, fields defined in `#preloads` are preloaded if that field is selected in the query. Occasionally, finer control may be needed to avoid preloading too much or incorrect content. @@ -1121,7 +1221,7 @@ available in the `Resolver` class as `parent`. To find the parent object in your `Presenter` class: -1. Add the parent object to the GraphQL `context` from within your resolver's `resolve` method: +1. Add the parent object to the GraphQL `context` from your resolver's `resolve` method: ```ruby def resolve(**args) @@ -1316,6 +1416,8 @@ Where an object has an `iid`, prefer to use the `full_path` or `group_path` of its parent in combination with its `iid` as arguments to identify an object rather than its `id`. +See also [Deprecate Global IDs](#deprecate-global-ids). + ### Fields In the most common situations, a mutation would return 2 fields: @@ -1327,7 +1429,7 @@ In the most common situations, a mutation would return 2 fields: By inheriting any new mutations from `Mutations::BaseMutation` the `errors` field is automatically added. A `clientMutationId` field is also added, this can be used by the client to identify the result of a -single mutation when multiple are performed within a single request. +single mutation when multiple are performed in a single request. ### The `resolve` method @@ -1447,7 +1549,7 @@ There are three states a mutation response can be in: #### Success In the happy path, errors *may* be returned, along with the anticipated payload, but -if everything was successful, then `errors` should be an empty array, since +if everything was successful, then `errors` should be an empty array, because there are no problems we need to inform the user of. ```javascript @@ -1524,7 +1626,7 @@ of errors should be treated as internal, and not shown to the user in specific detail. We need to inform the user when the mutation fails, but we do not need to -tell them why, since they cannot have caused it, and nothing they can do +tell them why, because they cannot have caused it, and nothing they can do fixes it, although we may offer to retry the mutation. #### Categorizing errors @@ -1544,7 +1646,7 @@ See also the [frontend GraphQL guide](../development/fe_guide/graphql.md#handlin ### Aliasing and deprecating mutations The `#mount_aliased_mutation` helper allows us to alias a mutation as -another name within `MutationType`. +another name in `MutationType`. For example, to alias a mutation called `FooMutation` as `BarMutation`: @@ -1565,7 +1667,7 @@ mount_aliased_mutation 'UpdateFoo', ``` Deprecated mutations should be added to `Types::DeprecatedMutations` and -tested for within the unit test of `Types::MutationType`. The merge request +tested for in the unit test of `Types::MutationType`. The merge request [!34798](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/34798) can be referred to as an example of this, including the method of testing deprecated aliased mutations. @@ -1591,7 +1693,7 @@ We cannot test subscriptions using GraphiQL, because they require an Action Cabl All fields under `Types::SubscriptionType` are subscriptions that clients can subscribe to. These fields require a subscription class, which is a descendant of `Subscriptions::BaseSubscription` and is stored under `app/graphql/subscriptions`. -The arguments required to subscribe and the fields that are returned are defined within the subscription class. Multiple fields can share +The arguments required to subscribe and the fields that are returned are defined in the subscription class. Multiple fields can share the same subscription class if they have the same arguments and return the same fields. This class runs during the initial subscription request and subsequent updates. You can read more about this in the @@ -1623,8 +1725,8 @@ as normal. Sometimes a mutation or resolver may accept a number of optional arguments, but we still want to validate that at least one of the optional arguments is provided. In this situation, consider using the `#ready?` -method within your mutation or resolver to provide the validation. The -`#ready?` method is called before any work is done within the +method in your mutation or resolver to provide the validation. The +`#ready?` method is called before any work is done in the `#resolve` method. Example: @@ -1698,7 +1800,7 @@ For speed, you should test most logic in unit tests instead of integration tests However, integration tests that check if data is returned verify the following additional items: -- The mutation is actually queryable within the schema (was mounted in `MutationType`). +- The mutation is actually queryable in the schema (was mounted in `MutationType`). - The data returned by a resolver or mutation correctly matches the [return types](https://graphql-ruby.org/fields/introduction.html#field-return-type) of the fields and resolves without errors. @@ -1846,7 +1948,7 @@ to protect server resources from overly ambitious or malicious queries. These values can be set as defaults and overridden in specific queries as needed. The complexity values can be set per object as well, and the final query complexity is evaluated based on how many objects are being returned. This is useful -for objects that are expensive (e.g. requiring Gitaly calls). +for objects that are expensive (such as requiring Gitaly calls). For example, a conditional complexity method in a resolver: diff --git a/doc/development/cicd/cicd_reference_documentation_guide.md b/doc/development/cicd/cicd_reference_documentation_guide.md index 14a313daba8..a6e4c3f8de8 100644 --- a/doc/development/cicd/cicd_reference_documentation_guide.md +++ b/doc/development/cicd/cicd_reference_documentation_guide.md @@ -4,7 +4,7 @@ group: Pipeline Execution info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments --- -# CI/CD YAML reference style guide +# CI/CD YAML reference style guide **(FREE)** The CI/CD YAML reference uses a standard style to make it easier to use and update. diff --git a/doc/development/cicd/index.md b/doc/development/cicd/index.md index 025d63f4a62..6e3cdb59fd8 100644 --- a/doc/development/cicd/index.md +++ b/doc/development/cicd/index.md @@ -5,7 +5,7 @@ info: To determine the technical writer assigned to the Stage/Group associated w type: index, concepts, howto --- -# CI/CD development documentation +# CI/CD development documentation **(FREE)** Development guides that are specific to CI/CD are listed here. diff --git a/lib/gitlab/database.rb b/lib/gitlab/database.rb index aa419d75df2..be2ee9380bf 100644 --- a/lib/gitlab/database.rb +++ b/lib/gitlab/database.rb @@ -333,6 +333,16 @@ module Gitlab end end + def self.dbname(ar_connection) + if ar_connection.respond_to?(:pool) && + ar_connection.pool.respond_to?(:db_config) && + ar_connection.pool.db_config.respond_to?(:database) + return ar_connection.pool.db_config.database + end + + 'unknown' + end + # inside_transaction? will return true if the caller is running within a transaction. Handles special cases # when running inside a test environment, where tests may be wrapped in transactions def self.inside_transaction? diff --git a/lib/gitlab/metrics/subscribers/active_record.rb b/lib/gitlab/metrics/subscribers/active_record.rb index 9f7884e1364..07480bcddee 100644 --- a/lib/gitlab/metrics/subscribers/active_record.rb +++ b/lib/gitlab/metrics/subscribers/active_record.rb @@ -72,6 +72,14 @@ module Gitlab DB_LOAD_BALANCING_DURATIONS.each do |duration| payload[duration] = ::Gitlab::SafeRequestStore[duration].to_f.round(3) end + + if Feature.enabled?(:multiple_database_metrics, default_enabled: :yaml) + ::Gitlab::SafeRequestStore[:duration_by_database]&.each do |dbname, duration_by_role| + duration_by_role.each do |db_role, duration| + payload[:"db_#{db_role}_#{dbname}_duration_s"] = duration.to_f.round(3) + end + end + end end end end @@ -93,9 +101,18 @@ module Gitlab buckets ::Gitlab::Metrics::Subscribers::ActiveRecord::SQL_DURATION_BUCKET end + return unless ::Gitlab::SafeRequestStore.active? + duration = event.duration / 1000.0 duration_key = "db_#{db_role}_duration_s".to_sym ::Gitlab::SafeRequestStore[duration_key] = (::Gitlab::SafeRequestStore[duration_key].presence || 0) + duration + + # Per database metrics + dbname = ::Gitlab::Database.dbname(event.payload[:connection]) + ::Gitlab::SafeRequestStore[:duration_by_database] ||= {} + ::Gitlab::SafeRequestStore[:duration_by_database][dbname] ||= {} + ::Gitlab::SafeRequestStore[:duration_by_database][dbname][db_role] ||= 0 + ::Gitlab::SafeRequestStore[:duration_by_database][dbname][db_role] += duration end def ignored_query?(payload) diff --git a/spec/lib/gitlab/database_spec.rb b/spec/lib/gitlab/database_spec.rb index 847f7ec2d74..2e7b2c344f1 100644 --- a/spec/lib/gitlab/database_spec.rb +++ b/spec/lib/gitlab/database_spec.rb @@ -414,6 +414,23 @@ RSpec.describe Gitlab::Database do end end + describe '.dbname' do + it 'returns the dbname for the connection' do + connection = ActiveRecord::Base.connection + + expect(described_class.dbname(connection)).to be_a(String) + expect(described_class.dbname(connection)).to eq(connection.pool.db_config.database) + end + + context 'when the pool is a NullPool' do + it 'returns unknown' do + connection = double(:active_record_connection, pool: ActiveRecord::ConnectionAdapters::NullPool.new) + + expect(described_class.dbname(connection)).to eq('unknown') + end + end + end + describe '#true_value' do it 'returns correct value' do expect(described_class.true_value).to eq "'t'" diff --git a/spec/lib/gitlab/metrics/subscribers/active_record_spec.rb b/spec/lib/gitlab/metrics/subscribers/active_record_spec.rb index cffa62c3a52..915f28ec6e9 100644 --- a/spec/lib/gitlab/metrics/subscribers/active_record_spec.rb +++ b/spec/lib/gitlab/metrics/subscribers/active_record_spec.rb @@ -7,7 +7,7 @@ RSpec.describe Gitlab::Metrics::Subscribers::ActiveRecord do let(:env) { {} } let(:subscriber) { described_class.new } - let(:connection) { double(:connection) } + let(:connection) { ActiveRecord::Base.connection } describe '#transaction' do let(:web_transaction) { double('Gitlab::Metrics::WebTransaction') } diff --git a/spec/lib/gitlab/sidekiq_logging/structured_logger_spec.rb b/spec/lib/gitlab/sidekiq_logging/structured_logger_spec.rb index 0d8b02fb72f..22deaca824a 100644 --- a/spec/lib/gitlab/sidekiq_logging/structured_logger_spec.rb +++ b/spec/lib/gitlab/sidekiq_logging/structured_logger_spec.rb @@ -298,6 +298,8 @@ RSpec.describe Gitlab::SidekiqLogging::StructuredLogger do allow(Gitlab::Database::LoadBalancing).to receive(:enable?).and_return(true) end + let(:dbname) { ::Gitlab::Database.dbname(ActiveRecord::Base.connection) } + let(:expected_end_payload_with_db) do expected_end_payload.merge( 'db_duration_s' => a_value >= 0.1, @@ -311,7 +313,8 @@ RSpec.describe Gitlab::SidekiqLogging::StructuredLogger do 'db_primary_count' => a_value >= 1, 'db_primary_cached_count' => 0, 'db_primary_wal_count' => 0, - 'db_primary_duration_s' => a_value > 0 + 'db_primary_duration_s' => a_value > 0, + "db_primary_#{dbname}_duration_s" => a_value > 0 ) end diff --git a/spec/support/shared_examples/metrics/active_record_subscriber_shared_examples.rb b/spec/support/shared_examples/metrics/active_record_subscriber_shared_examples.rb index 1b110ab02b5..cce9d584690 100644 --- a/spec/support/shared_examples/metrics/active_record_subscriber_shared_examples.rb +++ b/spec/support/shared_examples/metrics/active_record_subscriber_shared_examples.rb @@ -5,9 +5,10 @@ RSpec.shared_examples 'store ActiveRecord info in RequestStore' do |db_role| 2.times do Gitlab::WithRequestStore.with_request_store do subscriber.sql(event) + connection = event.payload[:connection] if db_role == :primary - expect(described_class.db_counter_payload).to eq( + expected = { db_count: record_query ? 1 : 0, db_write_count: record_write_query ? 1 : 0, db_cached_count: record_cached_query ? 1 : 0, @@ -19,9 +20,10 @@ RSpec.shared_examples 'store ActiveRecord info in RequestStore' do |db_role| db_replica_duration_s: 0.0, db_primary_wal_count: record_wal_query ? 1 : 0, db_replica_wal_count: 0 - ) + } + expected[:"db_primary_#{::Gitlab::Database.dbname(connection)}_duration_s"] = 0.002 if record_query elsif db_role == :replica - expect(described_class.db_counter_payload).to eq( + expected = { db_count: record_query ? 1 : 0, db_write_count: record_write_query ? 1 : 0, db_cached_count: record_cached_query ? 1 : 0, @@ -33,14 +35,32 @@ RSpec.shared_examples 'store ActiveRecord info in RequestStore' do |db_role| db_replica_duration_s: record_query ? 0.002 : 0, db_replica_wal_count: record_wal_query ? 1 : 0, db_primary_wal_count: 0 - ) + } + expected[:"db_replica_#{::Gitlab::Database.dbname(connection)}_duration_s"] = 0.002 if record_query else - expect(described_class.db_counter_payload).to eq( + expected = { db_count: record_query ? 1 : 0, db_write_count: record_write_query ? 1 : 0, db_cached_count: record_cached_query ? 1 : 0 - ) + } end + + expect(described_class.db_counter_payload).to eq(expected) + end + end + end + + context 'when multiple_database_metrics is disabled' do + before do + stub_feature_flags(multiple_database_metrics: false) + end + + it 'does not include per database metrics' do + Gitlab::WithRequestStore.with_request_store do + subscriber.sql(event) + connection = event.payload[:connection] + + expect(described_class.db_counter_payload).not_to include(:"db_replica_#{::Gitlab::Database.dbname(connection)}_duration_s") end end end