Skip to content
Snippets Groups Projects

Add HTTP integration GraphQL query

Closed Add HTTP integration GraphQL query
321674-allow-to-filter-alert-management-integrations-by-id into master
Closed Vitali Tatarintev requested to merge 321674-allow-to-filter-alert-management-integrations-by-id into master

What does this MR do?

Related to #321674 (closed)

Adds a new alertManagementHttpIntegration to fetch information about a single HTTP Integration by ID.

Why there is no alertManagementIntegration query similar to alertManagementHttpIntegration?

Alert Management Integration consists of two kinds: HTTP Integrations and Prometheus integration. The last can only be present in a single copy and I think there is no need to filter it by ID when it's already fetched from the alertManagementIntegrations query.

If we ignore filtering Prometheus integration in alertManagementIntegration, then alertManagementIntegration and alertManagementHttpIntegration will do exactly the same job.

One may argue, in the future, we may have more integration types and alertManagementIntegration is going to allow us to filter any of them by ID. I would suggest getting back to that discussion once we have more integration types.

GraphQL

Query

query Extract($fullPath: ID!, $id: AlertManagementHttpIntegrationID!) {
  project(fullPath: $fullPath) {
    alertManagementHttpIntegration(id: $id) {
      id
      active
      name
      payloadExample
      payloadAttributeMappings {
        fieldName
        label
        type
      }
    }
  }
}

Query variables

{
  "fullPath": "<PROJECT-FULL-PATH>",
  "id": "gid://gitlab/AlertManagement::HttpIntegration/<ID>"
}

Screenshots (strongly suggested)

Screenshot_2021-03-15_at_14.09.14

Does this MR meet the acceptance criteria?

Conformity

Availability and Testing

Security

If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:

  • [-] Label as security and @ mention @gitlab-com/gl-security/appsec
  • [-] The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
  • [-] Security reports checked/validated by a reviewer from the AppSec team
Edited by Vitali Tatarintev

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
app/graphql/types/project_type.rb
279 279 description: 'HTTP Integrations which can receive alerts for the project.',
280 280 resolver: Resolvers::AlertManagement::HttpIntegrationsResolver
281 281
282 field :alert_management_http_integration,
  • Alex Kalderimis
    Alex Kalderimis @alexkalderimis
    Contributor

    We already have a field on the ProjectType that seems suitable for this: Project.alertManagementHttpIntegrations (I notice we also have Project.alertManagementIntegrations - I am worried that we are accumulating several very similar fields on Project that might be confusing to users.

    Did you consider other approaches here, such as adding filter arguments to Project.alertManagementIntegrations?

  • Vitali Tatarintev
    Vitali Tatarintev @ck3g
    Author Maintainer

    I was considering two approaches here. The one with the separate Project.alertManagementHttpIntegration and another one is adding a new filtering argument to the Project.alertManagementHttpIntegrations.

    I chose the new field approach due to its simplicity and single responsibility.

    But I don't mind flipping the solution in favor of Project.alertManagementHttpIntegrations.

    I didn't add the field to Project.alertManagementIntegrations because (I was trying to explain that in the MR description) that field returns two kinds of integrations: HTTP Integrations and the Prometheus service.

    The Prometheus service in there is always a single record (or blank I guess). That, I think, makes filtering by ID is a little bit tricky, because we need to identify the type to which is that ID belongs. Then, if the ID belongs to Prometheus, it doesn't make sense to filter by its ID again, because it's always a single record. Here we can only filter by HTTP integrations, but I thought it's could be odd to dismiss one of the types.

    Project.alertManagementHttpIntegrations on the other hand is only focused on HTTP integration, which has a couple of specific pieces of information we need when we edit one of the integration's settings.

  • Alex Kalderimis
    Alex Kalderimis @alexkalderimis
    Contributor

    I didn't add the field to Project.alertManagementIntegrations because (I was trying to explain that in the MR description) that field returns two kinds of integrations: HTTP Integrations and the Prometheus service.

    that is correct, but it doesn't seem like a large problem. We can distinguish which kind of integration this is with id.model_class.

  • Vitali Tatarintev
    Vitali Tatarintev @ck3g
    Author Maintainer

    To filter by ID we need to get the ID. Get it id.model_class. Then check if the model class is ::AlertManagement::HttpIntegration.

    If it's an HttpIntegration we pass the integer ID to ::AlertManagement::HttpIntegrationsFinder to filter the HTTP Integrations. https://gitlab.com/gitlab-org/gitlab/-/blob/d3250ebe5506b66bb7d019841a1f66981456974b/app/graphql/resolvers/alert_management/integrations_resolver.rb#L25

    If it's not an HttpIntegration we compare the project.prometheus_service ID with the ID we have, if return the Prometheus service if IDs match.

    Did I understand that right?

  • Please register or sign in to reply
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
    • Please register or sign in to reply