Add latest changes from gitlab-org/gitlab@master
This commit is contained in:
parent
27f6da0ab2
commit
37066217ce
|
@ -188,6 +188,5 @@ class GraphqlController < ApplicationController
|
|||
|
||||
def logs
|
||||
RequestStore.store[:graphql_logs].to_a
|
||||
.map { |log| log.except(:duration_s, :query_string) }
|
||||
end
|
||||
end
|
||||
|
|
|
@ -10,11 +10,15 @@ class GitlabSchema < GraphQL::Schema
|
|||
DEFAULT_MAX_DEPTH = 15
|
||||
AUTHENTICATED_MAX_DEPTH = 20
|
||||
|
||||
# Tracers (order is important)
|
||||
use Gitlab::Graphql::Tracers::LoggerTracer
|
||||
use Gitlab::Graphql::GenericTracing # Old tracer which will be removed eventually
|
||||
use Gitlab::Graphql::Tracers::TimerTracer
|
||||
|
||||
use GraphQL::Subscriptions::ActionCableSubscriptions
|
||||
use GraphQL::Pagination::Connections
|
||||
use BatchLoader::GraphQL
|
||||
use Gitlab::Graphql::Pagination::Connections
|
||||
use Gitlab::Graphql::GenericTracing
|
||||
use Gitlab::Graphql::Timeout, max_seconds: Gitlab.config.gitlab.graphql_timeout
|
||||
|
||||
query_analyzer Gitlab::Graphql::QueryAnalyzers::LoggerAnalyzer.new
|
||||
|
|
|
@ -42,13 +42,11 @@ module Mutations
|
|||
required: false,
|
||||
description: 'Description of or notes for the contact.'
|
||||
|
||||
authorize :admin_contact
|
||||
authorize :admin_crm_contact
|
||||
|
||||
def resolve(args)
|
||||
group = authorized_find!(id: args[:group_id])
|
||||
|
||||
raise Gitlab::Graphql::Errors::ResourceNotAvailable, 'Feature disabled' unless Feature.enabled?(:customer_relations, group, default_enabled: :yaml)
|
||||
|
||||
set_organization!(args)
|
||||
result = ::CustomerRelations::Contacts::CreateService.new(group: group, current_user: current_user, params: args).execute
|
||||
{ contact: result.payload, errors: result.errors }
|
||||
|
|
|
@ -8,7 +8,7 @@ module Mutations
|
|||
|
||||
graphql_name 'CustomerRelationsContactUpdate'
|
||||
|
||||
authorize :admin_contact
|
||||
authorize :admin_crm_contact
|
||||
|
||||
field :contact,
|
||||
Types::CustomerRelations::ContactType,
|
||||
|
@ -48,8 +48,6 @@ module Mutations
|
|||
raise_resource_not_available_error! unless contact
|
||||
|
||||
group = contact.group
|
||||
raise Gitlab::Graphql::Errors::ResourceNotAvailable, 'Feature disabled' unless Feature.enabled?(:customer_relations, group, default_enabled: :yaml)
|
||||
|
||||
authorize!(group)
|
||||
|
||||
result = ::CustomerRelations::Contacts::UpdateService.new(group: group, current_user: current_user, params: args).execute(contact)
|
||||
|
|
|
@ -33,13 +33,11 @@ module Mutations
|
|||
required: false,
|
||||
description: 'Description of or notes for the organization.'
|
||||
|
||||
authorize :admin_organization
|
||||
authorize :admin_crm_organization
|
||||
|
||||
def resolve(args)
|
||||
group = authorized_find!(id: args[:group_id])
|
||||
|
||||
raise Gitlab::Graphql::Errors::ResourceNotAvailable, 'Feature disabled' unless Feature.enabled?(:customer_relations, group, default_enabled: :yaml)
|
||||
|
||||
result = ::CustomerRelations::Organizations::CreateService.new(group: group, current_user: current_user, params: args).execute
|
||||
{ organization: result.payload, errors: result.errors }
|
||||
end
|
||||
|
|
|
@ -8,7 +8,7 @@ module Mutations
|
|||
|
||||
graphql_name 'CustomerRelationsOrganizationUpdate'
|
||||
|
||||
authorize :admin_organization
|
||||
authorize :admin_crm_organization
|
||||
|
||||
field :organization,
|
||||
Types::CustomerRelations::OrganizationType,
|
||||
|
@ -39,8 +39,6 @@ module Mutations
|
|||
raise_resource_not_available_error! unless organization
|
||||
|
||||
group = organization.group
|
||||
raise Gitlab::Graphql::Errors::ResourceNotAvailable, 'Feature disabled' unless Feature.enabled?(:customer_relations, group, default_enabled: :yaml)
|
||||
|
||||
authorize!(group)
|
||||
|
||||
result = ::CustomerRelations::Organizations::UpdateService.new(group: group, current_user: current_user, params: args).execute(organization)
|
||||
|
|
|
@ -125,6 +125,10 @@ module Types
|
|||
field :path, GraphQL::Types::String, null: true,
|
||||
description: "Relative path to the pipeline's page."
|
||||
|
||||
field :commit, Types::CommitType, null: true,
|
||||
description: "Git commit of the pipeline.",
|
||||
calls_gitaly: true
|
||||
|
||||
field :commit_path, GraphQL::Types::String, null: true,
|
||||
description: 'Path to the commit that triggered the pipeline.'
|
||||
|
||||
|
|
|
@ -14,12 +14,19 @@ module Types
|
|||
description: 'SHA1 ID of the commit.'
|
||||
field :short_id, type: GraphQL::Types::String, null: false,
|
||||
description: 'Short SHA1 ID of the commit.'
|
||||
|
||||
field :title, type: GraphQL::Types::String, null: true, calls_gitaly: true,
|
||||
description: 'Title of the commit message.'
|
||||
markdown_field :title_html, null: true
|
||||
|
||||
field :full_title, type: GraphQL::Types::String, null: true, calls_gitaly: true,
|
||||
description: 'Full title of the commit message.'
|
||||
markdown_field :full_title_html, null: true
|
||||
|
||||
field :description, type: GraphQL::Types::String, null: true,
|
||||
description: 'Description of the commit message.'
|
||||
markdown_field :description_html, null: true
|
||||
|
||||
field :message, type: GraphQL::Types::String, null: true,
|
||||
description: 'Raw commit message.'
|
||||
field :authored_date, type: Types::TimeType, null: true,
|
||||
|
|
|
@ -5,7 +5,7 @@ module Types
|
|||
class ContactType < BaseObject
|
||||
graphql_name 'CustomerRelationsContact'
|
||||
|
||||
authorize :read_contact
|
||||
authorize :read_crm_contact
|
||||
|
||||
field :id,
|
||||
GraphQL::Types::ID,
|
||||
|
|
|
@ -5,7 +5,7 @@ module Types
|
|||
class OrganizationType < BaseObject
|
||||
graphql_name 'CustomerRelationsOrganization'
|
||||
|
||||
authorize :read_organization
|
||||
authorize :read_crm_organization
|
||||
|
||||
field :id,
|
||||
GraphQL::Types::ID,
|
||||
|
|
|
@ -91,7 +91,7 @@ class LegacyDiffNote < Note
|
|||
end
|
||||
|
||||
def skip_setting_st_diff?
|
||||
st_diff.present? && importing? && Feature.enabled?(:skip_legacy_diff_note_callback_on_import, default_enabled: :yaml)
|
||||
st_diff.present? && importing?
|
||||
end
|
||||
|
||||
def diff_for_line_code
|
||||
|
|
|
@ -75,6 +75,8 @@ class GroupPolicy < BasePolicy
|
|||
with_scope :subject
|
||||
condition(:has_project_with_service_desk_enabled) { @subject.has_project_with_service_desk_enabled? }
|
||||
|
||||
condition(:crm_enabled, score: 0, scope: :subject) { Feature.enabled?(:customer_relations, @subject) }
|
||||
|
||||
rule { can?(:read_group) & design_management_enabled }.policy do
|
||||
enable :read_design_activity
|
||||
end
|
||||
|
@ -113,8 +115,8 @@ class GroupPolicy < BasePolicy
|
|||
enable :read_group_member
|
||||
enable :read_custom_emoji
|
||||
enable :read_counts
|
||||
enable :read_organization
|
||||
enable :read_contact
|
||||
enable :read_crm_organization
|
||||
enable :read_crm_contact
|
||||
end
|
||||
|
||||
rule { ~public_group & ~has_access }.prevent :read_counts
|
||||
|
@ -134,8 +136,8 @@ class GroupPolicy < BasePolicy
|
|||
enable :create_package
|
||||
enable :create_package_settings
|
||||
enable :developer_access
|
||||
enable :admin_organization
|
||||
enable :admin_contact
|
||||
enable :admin_crm_organization
|
||||
enable :admin_crm_contact
|
||||
end
|
||||
|
||||
rule { reporter }.policy do
|
||||
|
@ -252,6 +254,13 @@ class GroupPolicy < BasePolicy
|
|||
enable :read_label
|
||||
end
|
||||
|
||||
rule { ~crm_enabled }.policy do
|
||||
prevent :read_crm_contact
|
||||
prevent :read_crm_organization
|
||||
prevent :admin_crm_contact
|
||||
prevent :admin_crm_organization
|
||||
end
|
||||
|
||||
def access_level(for_any_session: false)
|
||||
return GroupMember::NO_ACCESS if @user.nil?
|
||||
return GroupMember::NO_ACCESS unless user_is_user?
|
||||
|
|
|
@ -6,7 +6,7 @@ module CustomerRelations
|
|||
private
|
||||
|
||||
def allowed?
|
||||
current_user&.can?(:admin_contact, group)
|
||||
current_user&.can?(:admin_crm_contact, group)
|
||||
end
|
||||
|
||||
def error(message)
|
||||
|
|
|
@ -6,7 +6,7 @@ module CustomerRelations
|
|||
private
|
||||
|
||||
def allowed?
|
||||
current_user&.can?(:admin_organization, group)
|
||||
current_user&.can?(:admin_crm_organization, group)
|
||||
end
|
||||
|
||||
def error(message)
|
||||
|
|
|
@ -13,6 +13,9 @@
|
|||
.settings-content
|
||||
= render 'spam'
|
||||
|
||||
-# this partial is from JiHu, see details in https://gitlab.com/gitlab-jh/gitlab/-/merge_requests/135
|
||||
= render_if_exists 'admin/application_settings/content_validation_section'
|
||||
|
||||
%section.settings.as-abuse.no-animate#js-abuse-settings{ class: ('expanded' if expanded_by_default?) }
|
||||
.settings-header
|
||||
%h4
|
||||
|
|
|
@ -1,8 +0,0 @@
|
|||
---
|
||||
name: skip_legacy_diff_note_callback_on_import
|
||||
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/72897
|
||||
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/343666
|
||||
milestone: '14.5'
|
||||
type: development
|
||||
group: group::import
|
||||
default_enabled: false
|
|
@ -688,8 +688,8 @@ could do it with [HAProxy](https://www.haproxy.org/).
|
|||
The following IPs and names are used as an example:
|
||||
|
||||
- `10.6.0.21`: Patroni 1 (`patroni1.internal`)
|
||||
- `10.6.0.21`: Patroni 2 (`patroni2.internal`)
|
||||
- `10.6.0.22`: Patroni 3 (`patroni3.internal`)
|
||||
- `10.6.0.22`: Patroni 2 (`patroni2.internal`)
|
||||
- `10.6.0.23`: Patroni 3 (`patroni3.internal`)
|
||||
|
||||
```plaintext
|
||||
global
|
||||
|
@ -714,7 +714,7 @@ backend postgresql
|
|||
|
||||
server patroni1.internal 10.6.0.21:5432 maxconn 100 check port 8008
|
||||
server patroni2.internal 10.6.0.22:5432 maxconn 100 check port 8008
|
||||
server patroni3.internal 10.6.0.23.195:5432 maxconn 100 check port 8008
|
||||
server patroni3.internal 10.6.0.23:5432 maxconn 100 check port 8008
|
||||
```
|
||||
|
||||
Refer to your preferred Load Balancer's documentation for further guidance.
|
||||
|
|
|
@ -8740,6 +8740,8 @@ Represents a code quality degradation on the pipeline.
|
|||
| <a id="commitauthoreddate"></a>`authoredDate` | [`Time`](#time) | Timestamp of when the commit was authored. |
|
||||
| <a id="commitdescription"></a>`description` | [`String`](#string) | Description of the commit message. |
|
||||
| <a id="commitdescriptionhtml"></a>`descriptionHtml` | [`String`](#string) | The GitLab Flavored Markdown rendering of `description`. |
|
||||
| <a id="commitfulltitle"></a>`fullTitle` | [`String`](#string) | Full title of the commit message. |
|
||||
| <a id="commitfulltitlehtml"></a>`fullTitleHtml` | [`String`](#string) | The GitLab Flavored Markdown rendering of `full_title`. |
|
||||
| <a id="commitid"></a>`id` | [`ID!`](#id) | ID (global ID) of the commit. |
|
||||
| <a id="commitmessage"></a>`message` | [`String`](#string) | Raw commit message. |
|
||||
| <a id="commitsha"></a>`sha` | [`String!`](#string) | SHA1 ID of the commit. |
|
||||
|
@ -12433,6 +12435,7 @@ Represents a file or directory in the project repository that has been locked.
|
|||
| <a id="pipelinebeforesha"></a>`beforeSha` | [`String`](#string) | Base SHA of the source branch. |
|
||||
| <a id="pipelinecancelable"></a>`cancelable` | [`Boolean!`](#boolean) | Specifies if a pipeline can be canceled. |
|
||||
| <a id="pipelinecodequalityreports"></a>`codeQualityReports` | [`CodeQualityDegradationConnection`](#codequalitydegradationconnection) | Code Quality degradations reported on the pipeline. (see [Connections](#connections)) |
|
||||
| <a id="pipelinecommit"></a>`commit` | [`Commit`](#commit) | Git commit of the pipeline. |
|
||||
| <a id="pipelinecommitpath"></a>`commitPath` | [`String`](#string) | Path to the commit that triggered the pipeline. |
|
||||
| <a id="pipelinecommittedat"></a>`committedAt` | [`Time`](#time) | Timestamp of the pipeline's commit. |
|
||||
| <a id="pipelinecomplete"></a>`complete` | [`Boolean!`](#boolean) | Indicates if a pipeline is complete. |
|
||||
|
|
|
@ -4,184 +4,174 @@ group: Scalability
|
|||
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
|
||||
---
|
||||
|
||||
# Rails request apdex SLI
|
||||
# Rails request Apdex SLI
|
||||
|
||||
> [Introduced](https://gitlab.com/groups/gitlab-com/gl-infra/-/epics/525) in GitLab 14.4
|
||||
|
||||
NOTE:
|
||||
This SLI is not yet used in [error budgets for stage
|
||||
groups](../stage_group_dashboards.md#error-budget) or service
|
||||
monitoring. This is being worked on in [this
|
||||
project](https://gitlab.com/groups/gitlab-com/gl-infra/-/epics/573).
|
||||
This SLI is not yet used in
|
||||
[error budgets for stage groups](../stage_group_dashboards.md#error-budget)
|
||||
or service monitoring. To learn more about this work, read about how we are
|
||||
[incorporating custom SLIs](https://gitlab.com/groups/gitlab-com/gl-infra/-/epics/573)
|
||||
into error budgets and the service catalog.
|
||||
|
||||
The request apdex SLI (Service Level Indicator) is [an SLI defined in the application](index.md)
|
||||
that measures the duration of successful requests as an indicator for
|
||||
The request Apdex SLI (Service Level Indicator) is [an SLI defined in the application](index.md).
|
||||
It measures the duration of successful requests as an indicator for
|
||||
application performance. This includes the REST and GraphQL API, and the
|
||||
regular controller endpoints. It consists of these counters:
|
||||
|
||||
1. `gitlab_sli:rails_request_apdex:total`: This counter gets
|
||||
incremented for every request that did not result in a response
|
||||
with a 5xx status code. This means that slow failures don't get
|
||||
counted twice: The request is already counted in the error-SLI.
|
||||
with a `5xx` status code. It ensures slow failures are not
|
||||
counted twice, because the request is already counted in the error SLI.
|
||||
|
||||
1. `gitlab_sli:rails_request_apdex:success_total`: This counter gets
|
||||
incremented for every successful request that performed faster than
|
||||
the [defined target duration depending on the endpoint's
|
||||
urgency](#adjusting-request-urgency).
|
||||
the [defined target duration depending on the endpoint's urgency](#adjusting-request-urgency).
|
||||
|
||||
Both these counters are labeled with:
|
||||
|
||||
1. `endpoint_id`: The identification of the Rails Controller or the
|
||||
Grape-API endpoint
|
||||
Grape-API endpoint.
|
||||
|
||||
1. `feature_category`: The feature category specified for that
|
||||
controller or API endpoint.
|
||||
|
||||
## Request Apdex SLO
|
||||
|
||||
These counters can be combined into a success ratio, the objective for
|
||||
this ratio is defined in the service catalog per service:
|
||||
These counters can be combined into a success ratio. The objective for
|
||||
this ratio is defined in the service catalog per service. For this SLI to meet SLO,
|
||||
the ratio recorded must be higher than:
|
||||
|
||||
1. [Web: 0.998](https://gitlab.com/gitlab-com/runbooks/blob/master/metrics-catalog/services/web.jsonnet#L19)
|
||||
1. [API: 0.995](https://gitlab.com/gitlab-com/runbooks/blob/master/metrics-catalog/services/api.jsonnet#L19)
|
||||
1. [Git: 0.998](https://gitlab.com/gitlab-com/runbooks/blob/master/metrics-catalog/services/git.jsonnet#L22)
|
||||
|
||||
This means that for this SLI to meet SLO, the ratio recorded needs to
|
||||
be higher than those defined above.
|
||||
- [Web: 0.998](https://gitlab.com/gitlab-com/runbooks/blob/master/metrics-catalog/services/web.jsonnet#L19)
|
||||
- [API: 0.995](https://gitlab.com/gitlab-com/runbooks/blob/master/metrics-catalog/services/api.jsonnet#L19)
|
||||
- [Git: 0.998](https://gitlab.com/gitlab-com/runbooks/blob/master/metrics-catalog/services/git.jsonnet#L22)
|
||||
|
||||
For example: for the web-service, we want at least 99.8% of requests
|
||||
to be faster than their target duration.
|
||||
|
||||
These are the targets we use for alerting and service monitoring. So
|
||||
durations should be set keeping those into account. So we would not
|
||||
cause alerts. But the goal would be to set the urgency to a target
|
||||
that users would be satisfied with.
|
||||
We use these targets for alerting and service monitoring. Set durations taking
|
||||
these targets into account, so we don't cause alerts. The goal, however, is to
|
||||
set the urgency to a target that satisfies our users.
|
||||
|
||||
Both successful measurements and unsuccessful ones have an impact on the
|
||||
Both successful measurements and unsuccessful ones affect the
|
||||
error budget for stage groups.
|
||||
|
||||
## Adjusting request urgency
|
||||
|
||||
Not all endpoints perform the same type of work, so it is possible to
|
||||
define different urgency levels for different endpoints. An endpoint with a
|
||||
lower urgency can have a longer request duration than endpoints that
|
||||
are high urgency.
|
||||
lower urgency can have a longer request duration than endpoints with high urgency.
|
||||
|
||||
Long-running requests are more expensive for our
|
||||
infrastructure: while one request is being served, the thread remains
|
||||
occupied for the duration of that request. So nothing else can be handled by that
|
||||
thread. Because of Ruby's Global VM Lock, the thread might keep the
|
||||
Long-running requests are more expensive for our infrastructure. While serving
|
||||
one request, the thread remains occupied for the duration of that request. The thread
|
||||
can handle nothing else. Due to Ruby's Global VM Lock, the thread might keep the
|
||||
lock and stall other requests handled by the same Puma worker
|
||||
process. The request is in fact a noisy neighbor for other requests
|
||||
handled by the worker. This is why the upper bound for a target
|
||||
duration is capped at 5 seconds.
|
||||
process. The request is, in fact, a noisy neighbor for other requests
|
||||
handled by the worker. We cap the upper bound for a target duration at 5 seconds
|
||||
for this reason.
|
||||
|
||||
## Decreasing the urgency (setting a higher target duration)
|
||||
|
||||
Decreasing the urgency on an existing endpoint can be done on
|
||||
a case-by-case basis. Please take the following into account:
|
||||
You can decrease the urgency on an existing endpoint on
|
||||
a case-by-case basis. Take the following into account:
|
||||
|
||||
1. Apdex is about perceived performance, if a user is actively waiting
|
||||
1. Apdex is about perceived performance. If a user is actively waiting
|
||||
for the result of a request, waiting 5 seconds might not be
|
||||
acceptable. While if the endpoint is used by an automation
|
||||
requiring a lot of data, 5 seconds could be okay.
|
||||
acceptable. However, if the endpoint is used by an automation
|
||||
requiring a lot of data, 5 seconds could be acceptable.
|
||||
|
||||
A product manager can help to identify how an endpoint is used.
|
||||
|
||||
1. The workload for some endpoints can sometimes differ greatly
|
||||
depending on the parameters specified by the caller. The urgency
|
||||
needs to accommodate that. In some cases, it might be interesting to
|
||||
needs to accommodate those differences. In some cases, you could
|
||||
define a separate [application SLI](index.md#defining-a-new-sli)
|
||||
for what the endpoint is doing.
|
||||
|
||||
When the endpoints in certain cases turn into no-ops, making them
|
||||
very fast, we should ignore these fast requests when setting the
|
||||
target. For example, if the `MergeRequests::DraftsController` is
|
||||
hit for every merge request being viewed, but doesn't need to
|
||||
render anything in most cases, then we should pick the target that
|
||||
hit for every merge request being viewed, but rarely renders
|
||||
anything, then we should pick the target that
|
||||
would still accommodate the endpoint performing work.
|
||||
|
||||
1. Consider the dependent resources consumed by the endpoint. If the endpoint
|
||||
loads a lot of data from Gitaly or the database and this is causing
|
||||
it to not perform satisfactory. It could be better to optimize the
|
||||
loads a lot of data from Gitaly or the database, and this causes
|
||||
unsatisfactory performance, consider optimizing the
|
||||
way the data is loaded rather than increasing the target duration
|
||||
by lowering the urgency.
|
||||
|
||||
In cases like this, it might be appropriate to temporarily decrease
|
||||
In these cases, it might be appropriate to temporarily decrease
|
||||
urgency to make the endpoint meet SLO, if this is bearable for the
|
||||
infrastructure. In such cases, please link an issue from a code
|
||||
comment.
|
||||
infrastructure. In such cases, create a code comment linking to an issue.
|
||||
|
||||
If the endpoint consumes a lot of CPU time, we should also consider
|
||||
this: these kinds of requests are the kind of noisy neighbors we
|
||||
should try to keep as short as possible.
|
||||
|
||||
1. Traffic characteristics should also be taken into account: if the
|
||||
1. Traffic characteristics should also be taken into account. If the
|
||||
traffic to the endpoint is bursty, like CI traffic spinning up a
|
||||
big batch of jobs hitting the same endpoint, then having these
|
||||
endpoints take 5s is not acceptable from an infrastructure point of
|
||||
endpoints take five seconds is unacceptable from an infrastructure point of
|
||||
view. We cannot scale up the fleet fast enough to accommodate for
|
||||
the incoming slow requests alongside the regular traffic.
|
||||
|
||||
When lowering the urgency for an existing endpoint, please involve a
|
||||
[Scalability team member](https://about.gitlab.com/handbook/engineering/infrastructure/team/scalability/#team-members)
|
||||
in the review. We can use request rates and durations available in the
|
||||
logs to come up with a recommendation. Picking a threshold can be done
|
||||
using the same process as for [increasing
|
||||
urgency](#increasing-urgency-setting-a-lower-target-duration), picking
|
||||
a duration that is higher than the SLO for the service.
|
||||
logs to come up with a recommendation. You can pick a threshold
|
||||
using the same process as for
|
||||
[increasing urgency](#increasing-urgency-setting-a-lower-target-duration),
|
||||
picking a duration that is higher than the SLO for the service.
|
||||
|
||||
We shouldn't set the longest durations on endpoints in the merge
|
||||
requests that introduces them, since we don't yet have data to support
|
||||
requests that introduces them, because we don't yet have data to support
|
||||
the decision.
|
||||
|
||||
## Increasing urgency (setting a lower target duration)
|
||||
|
||||
When increasing the urgency, we need to make sure the endpoint
|
||||
When increasing the urgency, we must make sure the endpoint
|
||||
still meets SLO for the fleet that handles the request. You can use the
|
||||
information in the logs to determine this:
|
||||
information in the logs to check:
|
||||
|
||||
1. Open [this table in
|
||||
Kibana](https://log.gprd.gitlab.net/goto/bbb6465c68eb83642269e64a467df3df)
|
||||
1. Open [this table in Kibana](https://log.gprd.gitlab.net/goto/bbb6465c68eb83642269e64a467df3df)
|
||||
|
||||
1. The table loads information for the busiest endpoints by
|
||||
default. You can speed things up by adding a filter for
|
||||
`json.caller_id.keyword` and adding the identifier you're interested
|
||||
in (for example: `Projects::RawController#show`).
|
||||
default. To speed the response, add both:
|
||||
- A filter for `json.caller_id.keyword`.
|
||||
- The identifier you're interested in, such as `Projects::RawController#show`.
|
||||
|
||||
1. Check the [appropriate percentile duration](#request-apdex-slo) for
|
||||
the service the endpoint is handled by. The overall duration should
|
||||
be lower than the target you intend to set.
|
||||
the service handling the endpoint. The overall duration should
|
||||
be lower than your intended target.
|
||||
|
||||
1. If the overall duration is below the intended target. Please also
|
||||
check the peaks over time in [this
|
||||
graph](https://log.gprd.gitlab.net/goto/9319c4a402461d204d13f3a4924a89fc)
|
||||
1. If the overall duration is below the intended target, check the peaks over time
|
||||
in [this graph](https://log.gprd.gitlab.net/goto/9319c4a402461d204d13f3a4924a89fc)
|
||||
in Kibana. Here, the percentile in question should not peak above
|
||||
the target duration we want to set.
|
||||
|
||||
Since decreasing a threshold too much could result in alerts for the
|
||||
apdex degradation, please also involve a Scalability team member in
|
||||
As decreasing a threshold too much could result in alerts for the
|
||||
Apdex degradation, please also involve a Scalability team member in
|
||||
the merge request.
|
||||
|
||||
## How to adjust the urgency
|
||||
|
||||
The urgency can be specified similar to how endpoints [get a feature
|
||||
category](../feature_categorization/index.md).
|
||||
You can specify urgency similar to how endpoints
|
||||
[get a feature category](../feature_categorization/index.md). Endpoints without a
|
||||
specific target use the default urgency: 1s duration. These configurations
|
||||
are available:
|
||||
|
||||
For endpoints that don't have a specific target, the default urgency (1s duration) will be used.
|
||||
|
||||
The following configurations are available:
|
||||
|
||||
| Urgency | Duration in seconds | Notes |
|
||||
|----------|---------------------|-----------------------------------------------|
|
||||
| :high | 0.25s | |
|
||||
| :medium | 0.5s | |
|
||||
| :default | 1s | This is the default when nothing is specified |
|
||||
| :low | 5s | |
|
||||
| Urgency | Duration in seconds | Notes |
|
||||
|------------|---------------------|-----------------------------------------------|
|
||||
| `:high` | 0.25s | |
|
||||
| `:medium` | 0.5s | |
|
||||
| `:default` | 1s | The default when nothing is specified. |
|
||||
| `:low` | 5s | |
|
||||
|
||||
### Rails controller
|
||||
|
||||
An urgency can be specified for all actions in a controller like this:
|
||||
An urgency can be specified for all actions in a controller:
|
||||
|
||||
```ruby
|
||||
class Boards::ListsController < ApplicationController
|
||||
|
@ -189,8 +179,7 @@ class Boards::ListsController < ApplicationController
|
|||
end
|
||||
```
|
||||
|
||||
To specify the urgency also for certain actions in a controller, they
|
||||
can be specified like this:
|
||||
To also specify the urgency for certain actions in a controller:
|
||||
|
||||
```ruby
|
||||
class Boards::ListsController < ApplicationController
|
||||
|
@ -200,8 +189,7 @@ end
|
|||
|
||||
### Grape endpoints
|
||||
|
||||
To specify the urgency for an entire API class, this can be done as
|
||||
follows:
|
||||
To specify the urgency for an entire API class:
|
||||
|
||||
```ruby
|
||||
module API
|
||||
|
@ -211,8 +199,7 @@ module API
|
|||
end
|
||||
```
|
||||
|
||||
To specify the urgency also for certain actions in a API class, they
|
||||
can be specified like this:
|
||||
To specify the urgency also for certain actions in a API class:
|
||||
|
||||
```ruby
|
||||
module API
|
||||
|
@ -245,7 +232,7 @@ The endpoints for the SLI feed into a group's error budget based on the
|
|||
To know which endpoints are included for your group, you can see the
|
||||
request rates on the
|
||||
[group dashboard for your group](https://dashboards.gitlab.net/dashboards/f/stage-groups/stage-groups).
|
||||
In the **Budget Attribution** row, the **Puma apdex** log link shows you
|
||||
In the **Budget Attribution** row, the **Puma Apdex** log link shows you
|
||||
how many requests are not meeting a 1s or 5s target.
|
||||
|
||||
Learn more about the content of the dashboard in the documentation for
|
||||
|
|
|
@ -76,7 +76,9 @@ To enable the unauthenticated request rate limit:
|
|||
|
||||
## Use a custom rate limit response
|
||||
|
||||
A request that exceeds a rate limit returns a 429 response code and a
|
||||
> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/50693) in GitLab 13.8.
|
||||
|
||||
A request that exceeds a rate limit returns a `429` response code and a
|
||||
plain-text body, which by default is `Retry later`.
|
||||
|
||||
To use a custom response:
|
||||
|
@ -88,7 +90,7 @@ To use a custom response:
|
|||
|
||||
## Response headers
|
||||
|
||||
> [Introduced](https://gitlab.com/gitlab-com/gl-infra/scalability/-/issues/731) in GitLab 13.8, the `Rate-Limit` headers. `Retry-After` was introduced in an earlier version.
|
||||
> [Introduced](https://gitlab.com/gitlab-com/gl-infra/scalability/-/issues/731) in GitLab 13.8, the `RateLimit` headers. `Retry-After` was introduced in an earlier version.
|
||||
|
||||
When a client exceeds the associated rate limit, the following requests are
|
||||
blocked. The server may respond with rate-limiting information allowing the
|
||||
|
|
|
@ -80,7 +80,6 @@ module Gitlab
|
|||
cache: job[:cache],
|
||||
resource_group_key: job[:resource_group],
|
||||
scheduling_type: job[:scheduling_type],
|
||||
secrets: job[:secrets],
|
||||
options: {
|
||||
image: job[:image],
|
||||
services: job[:services],
|
||||
|
|
|
@ -10,15 +10,10 @@ module Gitlab
|
|||
ALL_ANALYZERS = [COMPLEXITY_ANALYZER, DEPTH_ANALYZER, FIELD_USAGE_ANALYZER].freeze
|
||||
|
||||
def initial_value(query)
|
||||
variables = process_variables(query.provided_variables)
|
||||
default_initial_values(query).merge({
|
||||
operation_name: query.operation_name,
|
||||
query_string: query.query_string,
|
||||
variables: variables
|
||||
})
|
||||
rescue StandardError => e
|
||||
Gitlab::ErrorTracking.track_and_raise_for_dev_exception(e)
|
||||
default_initial_values(query)
|
||||
{
|
||||
time_started: Gitlab::Metrics::System.monotonic_time,
|
||||
query: query
|
||||
}
|
||||
end
|
||||
|
||||
def call(memo, *)
|
||||
|
@ -28,25 +23,42 @@ module Gitlab
|
|||
def final_value(memo)
|
||||
return if memo.nil?
|
||||
|
||||
complexity, depth, field_usages = GraphQL::Analysis.analyze_query(memo[:query], ALL_ANALYZERS)
|
||||
query = memo[:query]
|
||||
complexity, depth, field_usages = GraphQL::Analysis.analyze_query(query, ALL_ANALYZERS)
|
||||
|
||||
memo[:depth] = depth
|
||||
memo[:complexity] = complexity
|
||||
# This duration is not the execution time of the
|
||||
# query but the execution time of the analyzer.
|
||||
memo[:duration_s] = duration(memo[:time_started]).round(1)
|
||||
memo[:duration_s] = duration(memo[:time_started])
|
||||
memo[:used_fields] = field_usages.first
|
||||
memo[:used_deprecated_fields] = field_usages.second
|
||||
|
||||
RequestStore.store[:graphql_logs] ||= []
|
||||
RequestStore.store[:graphql_logs] << memo
|
||||
GraphqlLogger.info(memo.except!(:time_started, :query))
|
||||
push_to_request_store(memo)
|
||||
|
||||
# This gl_analysis is included in the tracer log
|
||||
query.context[:gl_analysis] = memo.except!(:time_started, :query)
|
||||
rescue StandardError => e
|
||||
Gitlab::ErrorTracking.track_and_raise_for_dev_exception(e)
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
def push_to_request_store(memo)
|
||||
query = memo[:query]
|
||||
|
||||
# TODO: This RequestStore management is used to handle setting request wide metadata
|
||||
# to improve preexisting logging. We should handle this either with ApplicationContext
|
||||
# or in a separate tracer.
|
||||
# https://gitlab.com/gitlab-org/gitlab/-/issues/343802
|
||||
|
||||
RequestStore.store[:graphql_logs] ||= []
|
||||
RequestStore.store[:graphql_logs] << memo.except(:time_started, :duration_s, :query).merge({
|
||||
variables: process_variables(query.provided_variables),
|
||||
operation_name: query.operation_name
|
||||
})
|
||||
end
|
||||
|
||||
def process_variables(variables)
|
||||
filtered_variables = filter_sensitive_variables(variables)
|
||||
|
||||
|
@ -66,16 +78,6 @@ module Gitlab
|
|||
def duration(time_started)
|
||||
Gitlab::Metrics::System.monotonic_time - time_started
|
||||
end
|
||||
|
||||
def default_initial_values(query)
|
||||
{
|
||||
time_started: Gitlab::Metrics::System.monotonic_time,
|
||||
query_string: nil,
|
||||
query: query,
|
||||
variables: nil,
|
||||
duration_s: nil
|
||||
}
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
@ -0,0 +1,60 @@
|
|||
# frozen_string_literal: true
|
||||
|
||||
module Gitlab
|
||||
module Graphql
|
||||
module Tracers
|
||||
# This tracer writes logs for certain trace events.
|
||||
# It reads duration metadata written by TimerTracer.
|
||||
class LoggerTracer
|
||||
def self.use(schema)
|
||||
schema.tracer(self.new)
|
||||
end
|
||||
|
||||
def trace(key, data)
|
||||
result = yield
|
||||
|
||||
case key
|
||||
when "execute_query"
|
||||
log_execute_query(**data)
|
||||
end
|
||||
|
||||
result
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
def log_execute_query(query: nil, duration_s: 0)
|
||||
# execute_query should always have :query, but we're just being defensive
|
||||
return unless query
|
||||
|
||||
analysis_info = query.context[:gl_analysis]&.transform_keys { |key| "query_analysis.#{key}" }
|
||||
info = {
|
||||
trace_type: 'execute_query',
|
||||
query_fingerprint: query.fingerprint,
|
||||
duration_s: duration_s,
|
||||
operation_name: query.operation_name,
|
||||
operation_fingerprint: query.operation_fingerprint,
|
||||
is_mutation: query.mutation?,
|
||||
variables: clean_variables(query.provided_variables),
|
||||
query_string: query.query_string
|
||||
}
|
||||
|
||||
info.merge!(::Gitlab::ApplicationContext.current)
|
||||
info.merge!(analysis_info) if analysis_info
|
||||
|
||||
::Gitlab::GraphqlLogger.info(info)
|
||||
end
|
||||
|
||||
def query_variables_for_logging(query)
|
||||
clean_variables(query.provided_variables)
|
||||
end
|
||||
|
||||
def clean_variables(variables)
|
||||
ActiveSupport::ParameterFilter
|
||||
.new(::Rails.application.config.filter_parameters)
|
||||
.filter(variables)
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
|
@ -0,0 +1,31 @@
|
|||
# frozen_string_literal: true
|
||||
|
||||
module Gitlab
|
||||
module Graphql
|
||||
module Tracers
|
||||
# This graphql-ruby tracer records duration for trace events and merges
|
||||
# the duration into the trace event's metadata. This way, separate tracers
|
||||
# can all use the same duration information.
|
||||
#
|
||||
# NOTE: TimerTracer should be applied last **after** other tracers, so
|
||||
# that it runs first (similar to function composition)
|
||||
class TimerTracer
|
||||
def self.use(schema)
|
||||
schema.tracer(self.new)
|
||||
end
|
||||
|
||||
def trace(key, data)
|
||||
start_time = Gitlab::Metrics::System.monotonic_time
|
||||
|
||||
result = yield
|
||||
|
||||
duration_s = Gitlab::Metrics::System.monotonic_time - start_time
|
||||
|
||||
data[:duration_s] = duration_s
|
||||
|
||||
result
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
|
@ -24,8 +24,13 @@ module Gitlab
|
|||
else
|
||||
{}
|
||||
end
|
||||
when Hash, ActionController::Parameters
|
||||
when Hash
|
||||
ambiguous_param
|
||||
when ActionController::Parameters
|
||||
# We can and have to trust the "Parameters" because `graphql-ruby` handles this hash safely
|
||||
# Also, `graphql-ruby` uses hash-specific methods, for example `size`:
|
||||
# https://sourcegraph.com/github.com/rmosolgo/graphql-ruby@61232b03412df6685406fc46c414e11d3f447817/-/blob/lib/graphql/query.rb?L304
|
||||
ambiguous_param.to_unsafe_h
|
||||
when nil
|
||||
{}
|
||||
else
|
||||
|
|
|
@ -45,7 +45,7 @@ RSpec.describe Mutations::CustomerRelations::Contacts::Create do
|
|||
|
||||
it 'raises an error' do
|
||||
expect { resolve_mutation }.to raise_error(Gitlab::Graphql::Errors::ResourceNotAvailable)
|
||||
.with_message('Feature disabled')
|
||||
.with_message("The resource that you are attempting to access does not exist or you don't have permission to perform this action")
|
||||
end
|
||||
end
|
||||
|
||||
|
@ -97,5 +97,5 @@ RSpec.describe Mutations::CustomerRelations::Contacts::Create do
|
|||
end
|
||||
end
|
||||
|
||||
specify { expect(described_class).to require_graphql_authorizations(:admin_contact) }
|
||||
specify { expect(described_class).to require_graphql_authorizations(:admin_crm_contact) }
|
||||
end
|
||||
|
|
|
@ -65,11 +65,11 @@ RSpec.describe Mutations::CustomerRelations::Contacts::Update do
|
|||
|
||||
it 'raises an error' do
|
||||
expect { resolve_mutation }.to raise_error(Gitlab::Graphql::Errors::ResourceNotAvailable)
|
||||
.with_message('Feature disabled')
|
||||
.with_message("The resource that you are attempting to access does not exist or you don't have permission to perform this action")
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
specify { expect(described_class).to require_graphql_authorizations(:admin_contact) }
|
||||
specify { expect(described_class).to require_graphql_authorizations(:admin_crm_contact) }
|
||||
end
|
||||
|
|
|
@ -46,7 +46,7 @@ RSpec.describe Mutations::CustomerRelations::Organizations::Create do
|
|||
|
||||
it 'raises an error' do
|
||||
expect { resolve_mutation }.to raise_error(Gitlab::Graphql::Errors::ResourceNotAvailable)
|
||||
.with_message('Feature disabled')
|
||||
.with_message("The resource that you are attempting to access does not exist or you don't have permission to perform this action")
|
||||
end
|
||||
end
|
||||
|
||||
|
@ -69,5 +69,5 @@ RSpec.describe Mutations::CustomerRelations::Organizations::Create do
|
|||
end
|
||||
end
|
||||
|
||||
specify { expect(described_class).to require_graphql_authorizations(:admin_organization) }
|
||||
specify { expect(described_class).to require_graphql_authorizations(:admin_crm_organization) }
|
||||
end
|
||||
|
|
|
@ -63,11 +63,11 @@ RSpec.describe Mutations::CustomerRelations::Organizations::Update do
|
|||
|
||||
it 'raises an error' do
|
||||
expect { resolve_mutation }.to raise_error(Gitlab::Graphql::Errors::ResourceNotAvailable)
|
||||
.with_message('Feature disabled')
|
||||
.with_message("The resource that you are attempting to access does not exist or you don't have permission to perform this action")
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
specify { expect(described_class).to require_graphql_authorizations(:admin_organization) }
|
||||
specify { expect(described_class).to require_graphql_authorizations(:admin_crm_organization) }
|
||||
end
|
||||
|
|
|
@ -11,6 +11,8 @@ RSpec.describe Resolvers::TimelogResolver do
|
|||
let_it_be(:issue) { create(:issue, project: project) }
|
||||
let_it_be(:error_class) { Gitlab::Graphql::Errors::ArgumentError }
|
||||
|
||||
let(:timelogs) { resolve_timelogs(**args) }
|
||||
|
||||
specify do
|
||||
expect(described_class).to have_non_null_graphql_type(::Types::TimelogType.connection_type)
|
||||
end
|
||||
|
@ -24,8 +26,6 @@ RSpec.describe Resolvers::TimelogResolver do
|
|||
let(:args) { { start_time: 6.days.ago, end_time: 2.days.ago.noon } }
|
||||
|
||||
it 'finds all timelogs within given dates' do
|
||||
timelogs = resolve_timelogs(**args)
|
||||
|
||||
expect(timelogs).to contain_exactly(timelog1)
|
||||
end
|
||||
|
||||
|
@ -33,8 +33,6 @@ RSpec.describe Resolvers::TimelogResolver do
|
|||
let(:args) { {} }
|
||||
|
||||
it 'finds all timelogs' do
|
||||
timelogs = resolve_timelogs(**args)
|
||||
|
||||
expect(timelogs).to contain_exactly(timelog1, timelog2, timelog3)
|
||||
end
|
||||
end
|
||||
|
@ -43,8 +41,6 @@ RSpec.describe Resolvers::TimelogResolver do
|
|||
let(:args) { { start_time: 2.days.ago.noon } }
|
||||
|
||||
it 'finds timelogs after the start_time' do
|
||||
timelogs = resolve_timelogs(**args)
|
||||
|
||||
expect(timelogs).to contain_exactly(timelog2)
|
||||
end
|
||||
end
|
||||
|
@ -53,8 +49,6 @@ RSpec.describe Resolvers::TimelogResolver do
|
|||
let(:args) { { end_time: 2.days.ago.noon } }
|
||||
|
||||
it 'finds timelogs before the end_time' do
|
||||
timelogs = resolve_timelogs(**args)
|
||||
|
||||
expect(timelogs).to contain_exactly(timelog1, timelog3)
|
||||
end
|
||||
end
|
||||
|
@ -63,8 +57,6 @@ RSpec.describe Resolvers::TimelogResolver do
|
|||
let(:args) { { start_time: 6.days.ago, end_date: 2.days.ago } }
|
||||
|
||||
it 'finds timelogs until the end of day of end_date' do
|
||||
timelogs = resolve_timelogs(**args)
|
||||
|
||||
expect(timelogs).to contain_exactly(timelog1, timelog2)
|
||||
end
|
||||
end
|
||||
|
@ -73,8 +65,6 @@ RSpec.describe Resolvers::TimelogResolver do
|
|||
let(:args) { { start_date: 6.days.ago, end_time: 2.days.ago.noon } }
|
||||
|
||||
it 'finds all timelogs within start_date and end_time' do
|
||||
timelogs = resolve_timelogs(**args)
|
||||
|
||||
expect(timelogs).to contain_exactly(timelog1)
|
||||
end
|
||||
end
|
||||
|
@ -96,7 +86,7 @@ RSpec.describe Resolvers::TimelogResolver do
|
|||
let(:args) { { start_time: 6.days.ago, start_date: 6.days.ago } }
|
||||
|
||||
it 'returns correct error' do
|
||||
expect { resolve_timelogs(**args) }
|
||||
expect { timelogs }
|
||||
.to raise_error(error_class, /Provide either a start date or time, but not both/)
|
||||
end
|
||||
end
|
||||
|
@ -105,7 +95,7 @@ RSpec.describe Resolvers::TimelogResolver do
|
|||
let(:args) { { end_time: 2.days.ago, end_date: 2.days.ago } }
|
||||
|
||||
it 'returns correct error' do
|
||||
expect { resolve_timelogs(**args) }
|
||||
expect { timelogs }
|
||||
.to raise_error(error_class, /Provide either an end date or time, but not both/)
|
||||
end
|
||||
end
|
||||
|
@ -114,14 +104,14 @@ RSpec.describe Resolvers::TimelogResolver do
|
|||
let(:args) { { start_time: 2.days.ago, end_time: 6.days.ago } }
|
||||
|
||||
it 'returns correct error' do
|
||||
expect { resolve_timelogs(**args) }
|
||||
expect { timelogs }
|
||||
.to raise_error(error_class, /Start argument must be before End argument/)
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
shared_examples "with a group" do
|
||||
shared_examples 'with a group' do
|
||||
let_it_be(:short_time_ago) { 5.days.ago.beginning_of_day }
|
||||
let_it_be(:medium_time_ago) { 15.days.ago.beginning_of_day }
|
||||
|
||||
|
@ -141,8 +131,6 @@ RSpec.describe Resolvers::TimelogResolver do
|
|||
end
|
||||
|
||||
it 'finds all timelogs within given dates' do
|
||||
timelogs = resolve_timelogs(**args)
|
||||
|
||||
expect(timelogs).to contain_exactly(timelog1)
|
||||
end
|
||||
|
||||
|
@ -150,8 +138,6 @@ RSpec.describe Resolvers::TimelogResolver do
|
|||
let(:args) { { start_date: short_time_ago } }
|
||||
|
||||
it 'finds timelogs until the end of day of end_date' do
|
||||
timelogs = resolve_timelogs(**args)
|
||||
|
||||
expect(timelogs).to contain_exactly(timelog1, timelog2)
|
||||
end
|
||||
end
|
||||
|
@ -160,8 +146,6 @@ RSpec.describe Resolvers::TimelogResolver do
|
|||
let(:args) { { end_date: medium_time_ago } }
|
||||
|
||||
it 'finds timelogs until the end of day of end_date' do
|
||||
timelogs = resolve_timelogs(**args)
|
||||
|
||||
expect(timelogs).to contain_exactly(timelog3)
|
||||
end
|
||||
end
|
||||
|
@ -170,8 +154,6 @@ RSpec.describe Resolvers::TimelogResolver do
|
|||
let(:args) { { start_time: short_time_ago, end_date: short_time_ago } }
|
||||
|
||||
it 'finds timelogs until the end of day of end_date' do
|
||||
timelogs = resolve_timelogs(**args)
|
||||
|
||||
expect(timelogs).to contain_exactly(timelog1, timelog2)
|
||||
end
|
||||
end
|
||||
|
@ -180,8 +162,6 @@ RSpec.describe Resolvers::TimelogResolver do
|
|||
let(:args) { { start_date: short_time_ago, end_time: short_time_ago.noon } }
|
||||
|
||||
it 'finds all timelogs within start_date and end_time' do
|
||||
timelogs = resolve_timelogs(**args)
|
||||
|
||||
expect(timelogs).to contain_exactly(timelog1)
|
||||
end
|
||||
end
|
||||
|
@ -191,7 +171,7 @@ RSpec.describe Resolvers::TimelogResolver do
|
|||
let(:args) { { start_time: short_time_ago, start_date: short_time_ago } }
|
||||
|
||||
it 'returns correct error' do
|
||||
expect { resolve_timelogs(**args) }
|
||||
expect { timelogs }
|
||||
.to raise_error(error_class, /Provide either a start date or time, but not both/)
|
||||
end
|
||||
end
|
||||
|
@ -200,7 +180,7 @@ RSpec.describe Resolvers::TimelogResolver do
|
|||
let(:args) { { end_time: short_time_ago, end_date: short_time_ago } }
|
||||
|
||||
it 'returns correct error' do
|
||||
expect { resolve_timelogs(**args) }
|
||||
expect { timelogs }
|
||||
.to raise_error(error_class, /Provide either an end date or time, but not both/)
|
||||
end
|
||||
end
|
||||
|
@ -209,14 +189,14 @@ RSpec.describe Resolvers::TimelogResolver do
|
|||
let(:args) { { start_time: short_time_ago, end_time: medium_time_ago } }
|
||||
|
||||
it 'returns correct error' do
|
||||
expect { resolve_timelogs(**args) }
|
||||
expect { timelogs }
|
||||
.to raise_error(error_class, /Start argument must be before End argument/)
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
shared_examples "with a user" do
|
||||
shared_examples 'with a user' do
|
||||
let_it_be(:short_time_ago) { 5.days.ago.beginning_of_day }
|
||||
let_it_be(:medium_time_ago) { 15.days.ago.beginning_of_day }
|
||||
|
||||
|
@ -228,20 +208,18 @@ RSpec.describe Resolvers::TimelogResolver do
|
|||
let_it_be(:timelog3) { create(:merge_request_timelog, merge_request: merge_request, user: current_user) }
|
||||
|
||||
it 'blah' do
|
||||
timelogs = resolve_timelogs(**args)
|
||||
|
||||
expect(timelogs).to contain_exactly(timelog1, timelog3)
|
||||
end
|
||||
end
|
||||
|
||||
context "on a project" do
|
||||
context 'on a project' do
|
||||
let(:object) { project }
|
||||
let(:extra_args) { {} }
|
||||
|
||||
it_behaves_like 'with a project'
|
||||
end
|
||||
|
||||
context "with a project filter" do
|
||||
context 'with a project filter' do
|
||||
let(:object) { nil }
|
||||
let(:extra_args) { { project_id: project.to_global_id } }
|
||||
|
||||
|
@ -285,8 +263,6 @@ RSpec.describe Resolvers::TimelogResolver do
|
|||
let(:extra_args) { {} }
|
||||
|
||||
it 'pagination returns `default_max_page_size` and sets `has_next_page` true' do
|
||||
timelogs = resolve_timelogs(**args)
|
||||
|
||||
expect(timelogs.items.count).to be(100)
|
||||
expect(timelogs.has_next_page).to be(true)
|
||||
end
|
||||
|
@ -298,7 +274,7 @@ RSpec.describe Resolvers::TimelogResolver do
|
|||
let(:extra_args) { {} }
|
||||
|
||||
it 'returns correct error' do
|
||||
expect { resolve_timelogs(**args) }
|
||||
expect { timelogs }
|
||||
.to raise_error(error_class, /Provide at least one argument/)
|
||||
end
|
||||
end
|
||||
|
|
|
@ -13,7 +13,7 @@ RSpec.describe Types::Ci::PipelineType do
|
|||
duration queued_duration
|
||||
coverage created_at updated_at started_at finished_at committed_at
|
||||
stages user retryable cancelable jobs source_job job downstream
|
||||
upstream path project active user_permissions warnings commit_path uses_needs
|
||||
upstream path project active user_permissions warnings commit commit_path uses_needs
|
||||
test_report_summary test_suite ref
|
||||
]
|
||||
|
||||
|
|
|
@ -9,7 +9,7 @@ RSpec.describe GitlabSchema.types['Commit'] do
|
|||
|
||||
it 'contains attributes related to commit' do
|
||||
expect(described_class).to have_graphql_fields(
|
||||
:id, :sha, :short_id, :title, :description, :description_html, :message, :title_html, :authored_date,
|
||||
:id, :sha, :short_id, :title, :full_title, :full_title_html, :description, :description_html, :message, :title_html, :authored_date,
|
||||
:author_name, :author_gravatar, :author, :web_url, :web_path,
|
||||
:pipelines, :signature_html
|
||||
)
|
||||
|
|
|
@ -7,5 +7,5 @@ RSpec.describe GitlabSchema.types['CustomerRelationsContact'] do
|
|||
|
||||
it { expect(described_class.graphql_name).to eq('CustomerRelationsContact') }
|
||||
it { expect(described_class).to have_graphql_fields(fields) }
|
||||
it { expect(described_class).to require_graphql_authorizations(:read_contact) }
|
||||
it { expect(described_class).to require_graphql_authorizations(:read_crm_contact) }
|
||||
end
|
||||
|
|
|
@ -7,5 +7,5 @@ RSpec.describe GitlabSchema.types['CustomerRelationsOrganization'] do
|
|||
|
||||
it { expect(described_class.graphql_name).to eq('CustomerRelationsOrganization') }
|
||||
it { expect(described_class).to have_graphql_fields(fields) }
|
||||
it { expect(described_class).to require_graphql_authorizations(:read_organization) }
|
||||
it { expect(described_class).to require_graphql_authorizations(:read_crm_organization) }
|
||||
end
|
||||
|
|
|
@ -18,12 +18,6 @@ RSpec.describe Gitlab::Graphql::QueryAnalyzers::LoggerAnalyzer do
|
|||
GRAPHQL
|
||||
end
|
||||
|
||||
describe 'variables' do
|
||||
subject { initial_value.fetch(:variables) }
|
||||
|
||||
it { is_expected.to eq('{:body=>"[FILTERED]"}') }
|
||||
end
|
||||
|
||||
describe '#final_value' do
|
||||
let(:monotonic_time_before) { 42 }
|
||||
let(:monotonic_time_after) { 500 }
|
||||
|
@ -42,7 +36,14 @@ RSpec.describe Gitlab::Graphql::QueryAnalyzers::LoggerAnalyzer do
|
|||
|
||||
it 'inserts duration in seconds to memo and sets request store' do
|
||||
expect { final_value }.to change { memo[:duration_s] }.to(monotonic_time_duration)
|
||||
.and change { RequestStore.store[:graphql_logs] }.to([memo])
|
||||
.and change { RequestStore.store[:graphql_logs] }.to([{
|
||||
complexity: 4,
|
||||
depth: 2,
|
||||
operation_name: query.operation_name,
|
||||
used_deprecated_fields: [],
|
||||
used_fields: [],
|
||||
variables: { body: "[FILTERED]" }.to_s
|
||||
}])
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
@ -0,0 +1,52 @@
|
|||
# frozen_string_literal: true
|
||||
require "fast_spec_helper"
|
||||
require "support/graphql/fake_query_type"
|
||||
|
||||
RSpec.describe Gitlab::Graphql::Tracers::LoggerTracer do
|
||||
let(:dummy_schema) do
|
||||
Class.new(GraphQL::Schema) do
|
||||
# LoggerTracer depends on TimerTracer
|
||||
use Gitlab::Graphql::Tracers::LoggerTracer
|
||||
use Gitlab::Graphql::Tracers::TimerTracer
|
||||
|
||||
query_analyzer Gitlab::Graphql::QueryAnalyzers::LoggerAnalyzer.new
|
||||
|
||||
query Graphql::FakeQueryType
|
||||
end
|
||||
end
|
||||
|
||||
around do |example|
|
||||
Gitlab::ApplicationContext.with_context(caller_id: 'caller_a', feature_category: 'feature_a') do
|
||||
example.run
|
||||
end
|
||||
end
|
||||
|
||||
it "logs every query", :aggregate_failures do
|
||||
variables = { name: "Ada Lovelace" }
|
||||
query_string = 'query fooOperation($name: String) { helloWorld(message: $name) }'
|
||||
|
||||
# Build an actual query so we don't have to hardocde the "fingerprint" calculations
|
||||
query = GraphQL::Query.new(dummy_schema, query_string, variables: variables)
|
||||
|
||||
expect(::Gitlab::GraphqlLogger).to receive(:info).with({
|
||||
"correlation_id" => anything,
|
||||
"meta.caller_id" => "caller_a",
|
||||
"meta.feature_category" => "feature_a",
|
||||
"query_analysis.duration_s" => kind_of(Numeric),
|
||||
"query_analysis.complexity" => 1,
|
||||
"query_analysis.depth" => 1,
|
||||
"query_analysis.used_deprecated_fields" => [],
|
||||
"query_analysis.used_fields" => ["FakeQuery.helloWorld"],
|
||||
duration_s: be > 0,
|
||||
is_mutation: false,
|
||||
operation_fingerprint: query.operation_fingerprint,
|
||||
operation_name: 'fooOperation',
|
||||
query_fingerprint: query.fingerprint,
|
||||
query_string: query_string,
|
||||
trace_type: "execute_query",
|
||||
variables: variables
|
||||
})
|
||||
|
||||
dummy_schema.execute(query_string, variables: variables)
|
||||
end
|
||||
end
|
|
@ -0,0 +1,44 @@
|
|||
# frozen_string_literal: true
|
||||
require "fast_spec_helper"
|
||||
require "support/graphql/fake_tracer"
|
||||
require "support/graphql/fake_query_type"
|
||||
|
||||
RSpec.describe Gitlab::Graphql::Tracers::TimerTracer do
|
||||
let(:expected_duration) { 5 }
|
||||
let(:tracer_spy) { spy('tracer_spy') }
|
||||
let(:dummy_schema) do
|
||||
schema = Class.new(GraphQL::Schema) do
|
||||
use Gitlab::Graphql::Tracers::TimerTracer
|
||||
|
||||
query Graphql::FakeQueryType
|
||||
end
|
||||
|
||||
schema.tracer(Graphql::FakeTracer.new(lambda { |*args| tracer_spy.trace(*args) }))
|
||||
|
||||
schema
|
||||
end
|
||||
|
||||
before do
|
||||
current_time = 0
|
||||
allow(Gitlab::Metrics::System).to receive(:monotonic_time) do
|
||||
current_time += expected_duration
|
||||
end
|
||||
end
|
||||
|
||||
it "adds duration_s to the trace metadata", :aggregate_failures do
|
||||
query_string = "query fooOperation { helloWorld }"
|
||||
|
||||
dummy_schema.execute(query_string)
|
||||
|
||||
# "parse" and "execute_query" are just arbitrary trace events
|
||||
expect(tracer_spy).to have_received(:trace).with("parse", {
|
||||
duration_s: expected_duration,
|
||||
query_string: query_string
|
||||
})
|
||||
expect(tracer_spy).to have_received(:trace).with("execute_query", {
|
||||
# greater than expected duration because other calls made to `.monotonic_time` are outside our control
|
||||
duration_s: be >= expected_duration,
|
||||
query: instance_of(GraphQL::Query)
|
||||
})
|
||||
end
|
||||
end
|
|
@ -36,18 +36,6 @@ RSpec.describe LegacyDiffNote do
|
|||
expect(note.st_diff).to eq('_st_diff_')
|
||||
end
|
||||
|
||||
context 'when feature flag is false' do
|
||||
before do
|
||||
stub_feature_flags(skip_legacy_diff_note_callback_on_import: false)
|
||||
end
|
||||
|
||||
it 'updates st_diff' do
|
||||
note.save!(validate: false)
|
||||
|
||||
expect(note.st_diff).to eq({})
|
||||
end
|
||||
end
|
||||
|
||||
context 'when st_diff is blank' do
|
||||
before do
|
||||
note.st_diff = nil
|
||||
|
|
|
@ -11,8 +11,8 @@ RSpec.describe GroupPolicy do
|
|||
|
||||
it do
|
||||
expect_allowed(:read_group)
|
||||
expect_allowed(:read_organization)
|
||||
expect_allowed(:read_contact)
|
||||
expect_allowed(:read_crm_organization)
|
||||
expect_allowed(:read_crm_contact)
|
||||
expect_allowed(:read_counts)
|
||||
expect_allowed(*read_group_permissions)
|
||||
expect_disallowed(:upload_file)
|
||||
|
@ -33,8 +33,8 @@ RSpec.describe GroupPolicy do
|
|||
end
|
||||
|
||||
it { expect_disallowed(:read_group) }
|
||||
it { expect_disallowed(:read_organization) }
|
||||
it { expect_disallowed(:read_contact) }
|
||||
it { expect_disallowed(:read_crm_organization) }
|
||||
it { expect_disallowed(:read_crm_contact) }
|
||||
it { expect_disallowed(:read_counts) }
|
||||
it { expect_disallowed(*read_group_permissions) }
|
||||
end
|
||||
|
@ -48,8 +48,8 @@ RSpec.describe GroupPolicy do
|
|||
end
|
||||
|
||||
it { expect_disallowed(:read_group) }
|
||||
it { expect_disallowed(:read_organization) }
|
||||
it { expect_disallowed(:read_contact) }
|
||||
it { expect_disallowed(:read_crm_organization) }
|
||||
it { expect_disallowed(:read_crm_contact) }
|
||||
it { expect_disallowed(:read_counts) }
|
||||
it { expect_disallowed(*read_group_permissions) }
|
||||
end
|
||||
|
@ -933,8 +933,8 @@ RSpec.describe GroupPolicy do
|
|||
|
||||
it { is_expected.to be_allowed(:read_package) }
|
||||
it { is_expected.to be_allowed(:read_group) }
|
||||
it { is_expected.to be_allowed(:read_organization) }
|
||||
it { is_expected.to be_allowed(:read_contact) }
|
||||
it { is_expected.to be_allowed(:read_crm_organization) }
|
||||
it { is_expected.to be_allowed(:read_crm_contact) }
|
||||
it { is_expected.to be_disallowed(:create_package) }
|
||||
end
|
||||
|
||||
|
@ -944,8 +944,8 @@ RSpec.describe GroupPolicy do
|
|||
it { is_expected.to be_allowed(:create_package) }
|
||||
it { is_expected.to be_allowed(:read_package) }
|
||||
it { is_expected.to be_allowed(:read_group) }
|
||||
it { is_expected.to be_allowed(:read_organization) }
|
||||
it { is_expected.to be_allowed(:read_contact) }
|
||||
it { is_expected.to be_allowed(:read_crm_organization) }
|
||||
it { is_expected.to be_allowed(:read_crm_contact) }
|
||||
it { is_expected.to be_disallowed(:destroy_package) }
|
||||
end
|
||||
|
||||
|
@ -1032,4 +1032,17 @@ RSpec.describe GroupPolicy do
|
|||
it { is_expected.to be_disallowed(:update_runners_registration_token) }
|
||||
end
|
||||
end
|
||||
|
||||
context 'with customer_relations feature flag disabled' do
|
||||
let(:current_user) { owner }
|
||||
|
||||
before do
|
||||
stub_feature_flags(customer_relations: false)
|
||||
end
|
||||
|
||||
it { is_expected.to be_disallowed(:read_crm_contact) }
|
||||
it { is_expected.to be_disallowed(:read_crm_organization) }
|
||||
it { is_expected.to be_disallowed(:admin_crm_contact) }
|
||||
it { is_expected.to be_disallowed(:admin_crm_organization) }
|
||||
end
|
||||
end
|
||||
|
|
|
@ -190,19 +190,18 @@ RSpec.describe 'GitlabSchema configurations' do
|
|||
let(:query) { File.read(Rails.root.join('spec/fixtures/api/graphql/introspection.graphql')) }
|
||||
|
||||
it 'logs the query complexity and depth' do
|
||||
analyzer_memo = {
|
||||
query_string: query,
|
||||
variables: {}.to_s,
|
||||
complexity: 181,
|
||||
depth: 13,
|
||||
duration_s: 7,
|
||||
operation_name: 'IntrospectionQuery',
|
||||
used_fields: an_instance_of(Array),
|
||||
used_deprecated_fields: an_instance_of(Array)
|
||||
}
|
||||
|
||||
expect_any_instance_of(Gitlab::Graphql::QueryAnalyzers::LoggerAnalyzer).to receive(:duration).and_return(7)
|
||||
expect(Gitlab::GraphqlLogger).to receive(:info).with(analyzer_memo)
|
||||
|
||||
expect(Gitlab::GraphqlLogger).to receive(:info).with(
|
||||
hash_including(
|
||||
trace_type: 'execute_query',
|
||||
"query_analysis.duration_s" => 7,
|
||||
"query_analysis.complexity" => 181,
|
||||
"query_analysis.depth" => 13,
|
||||
"query_analysis.used_deprecated_fields" => an_instance_of(Array),
|
||||
"query_analysis.used_fields" => an_instance_of(Array)
|
||||
)
|
||||
)
|
||||
|
||||
post_graphql(query, current_user: nil)
|
||||
end
|
||||
|
|
|
@ -12,21 +12,33 @@ RSpec.describe 'GraphQL' do
|
|||
|
||||
describe 'logging' do
|
||||
shared_examples 'logging a graphql query' do
|
||||
let(:expected_params) do
|
||||
let(:expected_execute_query_log) do
|
||||
{
|
||||
query_string: query,
|
||||
variables: variables.to_s,
|
||||
duration_s: anything,
|
||||
"correlation_id" => kind_of(String),
|
||||
"meta.caller_id" => "GraphqlController#execute",
|
||||
"meta.client_id" => kind_of(String),
|
||||
"meta.feature_category" => "not_owned",
|
||||
"meta.remote_ip" => kind_of(String),
|
||||
"query_analysis.duration_s" => kind_of(Numeric),
|
||||
"query_analysis.depth" => 1,
|
||||
"query_analysis.complexity" => 1,
|
||||
"query_analysis.used_fields" => ['Query.echo'],
|
||||
"query_analysis.used_deprecated_fields" => [],
|
||||
# query_fingerprint starts with operation name
|
||||
query_fingerprint: %r{^anonymous\/},
|
||||
duration_s: kind_of(Numeric),
|
||||
trace_type: 'execute_query',
|
||||
operation_name: nil,
|
||||
depth: 1,
|
||||
complexity: 1,
|
||||
used_fields: ['Query.echo'],
|
||||
used_deprecated_fields: []
|
||||
# operation_fingerprint starts with operation name
|
||||
operation_fingerprint: %r{^anonymous\/},
|
||||
is_mutation: false,
|
||||
variables: variables,
|
||||
query_string: query
|
||||
}
|
||||
end
|
||||
|
||||
it 'logs a query with the expected params' do
|
||||
expect(Gitlab::GraphqlLogger).to receive(:info).with(expected_params).once
|
||||
expect(Gitlab::GraphqlLogger).to receive(:info).with(expected_execute_query_log).once
|
||||
|
||||
post_graphql(query, variables: variables)
|
||||
end
|
||||
|
|
|
@ -0,0 +1,15 @@
|
|||
# frozen_string_literal: true
|
||||
|
||||
module Graphql
|
||||
class FakeQueryType < Types::BaseObject
|
||||
graphql_name 'FakeQuery'
|
||||
|
||||
field :hello_world, String, null: true do
|
||||
argument :message, String, required: false
|
||||
end
|
||||
|
||||
def hello_world(message: "world")
|
||||
"Hello #{message}!"
|
||||
end
|
||||
end
|
||||
end
|
|
@ -0,0 +1,15 @@
|
|||
# frozen_string_literal: true
|
||||
|
||||
module Graphql
|
||||
class FakeTracer
|
||||
def initialize(trace_callback)
|
||||
@trace_callback = trace_callback
|
||||
end
|
||||
|
||||
def trace(*args)
|
||||
@trace_callback.call(*args)
|
||||
|
||||
yield
|
||||
end
|
||||
end
|
||||
end
|
Loading…
Reference in New Issue