Merge branch 'datadog-archive-trace-webhook' into 'master'

New hook type to send log links to Datadog

See merge request gitlab-org/gitlab!74725

.rubocop_todo/rails/include_url_helper.yml

@@ -7,7 +7,6 @@ Rails/IncludeUrlHelper:
   - app/models/integrations/campfire.rb
   - app/models/integrations/confluence.rb
   - app/models/integrations/custom_issue_tracker.rb
-  - app/models/integrations/datadog.rb
   - app/models/integrations/discord.rb
   - app/models/integrations/ewm.rb
   - app/models/integrations/external_wiki.rb

app/controllers/concerns/integrations/params.rb

@@ -11,6 +11,7 @@ module Integrations
       :api_key,
       :api_token,
       :api_url,
+      :archive_trace_events,
       :bamboo_url,
       :branches_to_be_notified,
       :labels_to_be_notified,

app/models/integration.rb

@@ -92,6 +92,7 @@ class Integration < ApplicationRecord
   scope :note_hooks, -> { where(note_events: true, active: true) }
   scope :confidential_note_hooks, -> { where(confidential_note_events: true, active: true) }
   scope :job_hooks, -> { where(job_events: true, active: true) }
+  scope :archive_trace_hooks, -> { where(archive_trace_events: true, active: true) }
   scope :pipeline_hooks, -> { where(pipeline_events: true, active: true) }
   scope :wiki_page_hooks, -> { where(wiki_page_events: true, active: true) }
   scope :deployment_hooks, -> { where(deployment_events: true, active: true) }

app/models/integrations/datadog.rb

@@ -2,7 +2,6 @@
 
 module Integrations
   class Datadog < Integration
-    include ActionView::Helpers::UrlHelper
     include HasWebHook
     extend Gitlab::Utils::Override
 
@@ -34,12 +33,21 @@ module Integrations
       SUPPORTED_EVENTS
     end
 
+    def supported_events
+      events = super
+
+      return events + ['archive_trace'] if Feature.enabled?(:datadog_integration_logs_collection, parent)
+
+      events
+    end
+
     def self.default_test_event
       'pipeline'
     end
 
     def configurable_events
       [] # do not allow to opt out of required hooks
+      # archive_trace is opt-in but we handle it with a more detailed field below
     end
 
     def title
@@ -51,7 +59,11 @@ module Integrations
     end
 
     def help
-      docs_link = link_to s_('DatadogIntegration|How do I set up this integration?'), Rails.application.routes.url_helpers.help_page_url('integration/datadog'), target: '_blank', rel: 'noopener noreferrer'
+      docs_link = ActionController::Base.helpers.link_to(
+        s_('DatadogIntegration|How do I set up this integration?'),
+        Rails.application.routes.url_helpers.help_page_url('integration/datadog'),
+        target: '_blank', rel: 'noopener noreferrer'
+      )
       s_('DatadogIntegration|Send CI/CD pipeline information to Datadog to monitor for job failures and troubleshoot performance issues. %{docs_link}').html_safe % { docs_link: docs_link.html_safe }
     end
 
@@ -60,7 +72,7 @@ module Integrations
     end
 
     def fields
-      [
+      f = [
         {
           type: 'text',
           name: 'datadog_site',
@@ -93,7 +105,21 @@ module Integrations
             linkClose: '</a>'.html_safe
           },
           required: true
-        },
+        }
+      ]
+
+      if Feature.enabled?(:datadog_integration_logs_collection, parent)
+        f.append({
+          type: 'checkbox',
+          name: 'archive_trace_events',
+          title: s_('Logs'),
+          checkbox_label: s_('Enable logs collection'),
+          help: s_('When enabled, job logs are collected by Datadog and displayed along with pipeline execution traces.'),
+          required: false
+        })
+      end
+
+      f += [
         {
           type: 'text',
           name: 'datadog_service',
@@ -116,6 +142,8 @@ module Integrations
           }
         }
       ]
+
+      f
     end
 
     override :hook_url
@@ -136,8 +164,7 @@ module Integrations
       object_kind = 'job' if object_kind == 'build'
       return unless supported_events.include?(object_kind)
 
-      data = data.with_retried_builds if data.respond_to?(:with_retried_builds)
-
+      data = hook_data(data, object_kind)
       execute_web_hook!(data, "#{object_kind} hook")
     end
 
@@ -158,5 +185,13 @@ module Integrations
       # US3 needs to keep a prefix but other datacenters cannot have the listed "app" prefix
       datadog_site.delete_prefix("app.")
     end
+
+    def hook_data(data, object_kind)
+      if object_kind == 'pipeline' && data.respond_to?(:with_retried_builds)
+        return data.with_retried_builds
+      end
+
+      data
+    end
   end
 end

app/services/ci/archive_trace_service.rb

@@ -27,6 +27,10 @@ module Ci
       job.trace.archive!
       job.remove_pending_state!
 
+      if Feature.enabled?(:datadog_integration_logs_collection, job.project) && job.job_artifacts_trace.present?
+        job.project.execute_integrations(Gitlab::DataBuilder::ArchiveTrace.build(job), :archive_trace_hooks)
+      end
+
       # TODO: Remove this logging once we confirmed new live trace architecture is functional.
       # See https://gitlab.com/gitlab-com/gl-infra/infrastructure/issues/4667.
       unless job.has_archived_trace?

config/feature_flags/development/datadog_integration_logs_collection.yml

+---
+name: datadog_integration_logs_collection
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/74725
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/346339
+milestone: '14.8'
+type: development
+group: group::integrations
+default_enabled: false

db/migrate/20211026124336_add_archive_trace_events_to_integrations.rb

+# frozen_string_literal: true
+
+class AddArchiveTraceEventsToIntegrations < Gitlab::Database::Migration[1.0]
+  def change
+    add_column :integrations, :archive_trace_events, :boolean, null: false, default: false
+  end
+end

db/schema_migrations/20211026124336

+a6807d2c17c4efdc759f39101856d7a082ae4d531ca3ced525de10e3de808b9d
\ No newline at end of file

db/structure.sql

@@ -15273,6 +15273,7 @@ CREATE TABLE integrations (
     group_id bigint,
     type_new text,
     vulnerability_events boolean DEFAULT false NOT NULL,
+    archive_trace_events boolean DEFAULT false NOT NULL,
     CONSTRAINT check_a948a0aa7e CHECK ((char_length(type_new) <= 255))
 );
 

doc/api/integrations.md

@@ -314,13 +314,15 @@ PUT /projects/:id/integrations/datadog
 
 Parameters:
 
-| Parameter | Type | Required | Description |
-| --------- | ---- | -------- | ----------- |
-| `api_key`         | string | true  | API key used for authentication with Datadog |
-| `api_url`         | string | false | (Advanced) Define the full URL for your Datadog site directly |
-| `datadog_site`    | string | false | Choose the Datadog site to send data to. Set to `datadoghq.eu` to send data to the EU site |
-| `datadog_service` | string | false | Name of this GitLab instance that all data will be tagged with |
-| `datadog_env`     | string | false | The environment tag that traces will be tagged with |
+| Parameter              | Type    | Required | Description |
+| ---------------------- | ------- | -------- | ----------- |
+| `api_key`              | string  | true     | API key used for authentication with Datadog |
+| `api_url`              | string  | false    | (Advanced) The full URL for your Datadog site |
+<!-- | `archive_trace_events` | boolean | false    | When enabled, job logs are collected by Datadog and displayed along with pipeline execution traces ([introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/346339) in GitLab 14.7) | -->
+<!-- TODO: uncomment the archive_trace_events field once :datadog_integration_logs_collection is rolled out. Rollout issue: https://gitlab.com/gitlab-org/gitlab/-/issues/346339 -->
+| `datadog_env`          | string  | false    | For self-managed deployments, set the env% tag for all the data sent to Datadog. |
+| `datadog_service`      | string  | false    | Tag all data from this GitLab instance in Datadog. Useful when managing several self-managed deployments |
+| `datadog_site`         | string  | false    | The Datadog site to send data to. To send data to the EU site, use `datadoghq.eu` |
 
 ### Disable Datadog integration
 

doc/integration/datadog.md

@@ -32,9 +32,11 @@ project, group, or instance level:
 1. Scroll to **Add an integration**, and select **Datadog**.
 1. Select **Active** to enable the integration.
 1. Specify the [**Datadog site**](https://docs.datadoghq.com/getting_started/site/) to send data to.
+1. Provide your Datadog **API key**.
+<!-- 1. Optional. Select **Enable logs collection** to enable logs collection for the output of jobs. ([Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/346339) in GitLab 14.8.) -->
+<!-- TODO: uncomment the archive_trace_events field once :datadog_integration_logs_collection is rolled out. Rollout issue: https://gitlab.com/gitlab-org/gitlab/-/issues/346339 -->
 1. Optional. To override the API URL used to send data directly, provide an **API URL**.
    Used only in advanced scenarios.
-1. Provide your Datadog **API key**.
 1. Optional. If you use more than one GitLab instance, provide a unique **Service** name
    to differentiate between your GitLab instances.
 1. Optional. If you use groups of GitLab instances (such as staging and production

lib/api/helpers/integrations_helpers.rb

@@ -314,25 +314,33 @@ module API
               required: false,
               name: :datadog_site,
               type: String,
-              desc: 'Choose the Datadog site to send data to. Set to "datadoghq.eu" to send data to the EU site'
+              desc: 'The Datadog site to send data to. To send data to the EU site, use datadoghq.eu'
             },
             {
               required: false,
               name: :api_url,
               type: String,
-              desc: '(Advanced) Define the full URL for your Datadog site directly'
+              desc: '(Advanced) The full URL for your Datadog site'
             },
+            # TODO: uncomment this field once :datadog_integration_logs_collection is rolled out
+            # https://gitlab.com/gitlab-org/gitlab/-/issues/346339
+            # {
+            #   required: false,
+            #   name: :archive_trace_events,
+            #   type: Boolean,
+            #   desc: 'When enabled, job logs will be collected by Datadog and shown along pipeline execution traces'
+            # },
             {
               required: false,
               name: :datadog_service,
               type: String,
-              desc: 'Name of this GitLab instance that all data will be tagged with'
+              desc: 'Tag all data from this GitLab instance in Datadog. Useful when managing several self-managed deployments'
             },
             {
               required: false,
               name: :datadog_env,
               type: String,
-              desc: 'The environment tag that traces will be tagged with'
+              desc: 'For self-managed deployments, set the env tag for all the data sent to Datadog. How do I use tags?'
             }
           ],
           'discord' => [

lib/api/integrations.rb

@@ -111,7 +111,14 @@ module API
           integration = user_project.find_or_initialize_integration(params[:slug].underscore)
 
           destroy_conditionally!(integration) do
-            attrs = integration_attributes(integration).index_with { nil }.merge(active: false)
+            attrs = integration_attributes(integration).index_with do |attr|
+              column = integration.column_for_attribute(attr)
+              if column.is_a?(ActiveRecord::ConnectionAdapters::NullColumn)
+                nil
+              else
+                column.default
+              end
+            end.merge(active: false)
 
             render_api_error!('400 Bad Request', 400) unless integration.update(attrs)
           end

lib/gitlab/data_builder/archive_trace.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module DataBuilder
+    module ArchiveTrace
+      extend self
+
+      def build(job)
+        {
+          object_kind: 'archive_trace',
+          trace_url: job.job_artifacts_trace.file.url,
+          build_id: job.id,
+          pipeline_id: job.pipeline_id,
+          project: job.project.hook_attrs
+        }
+      end
+    end
+  end
+end

locale/gitlab.pot

@@ -13234,6 +13234,9 @@ msgstr ""
 msgid "Enable kuromoji custom analyzer: Search"
 msgstr ""
 
+msgid "Enable logs collection"
+msgstr ""
+
 msgid "Enable maintenance mode"
 msgstr ""
 
@@ -40193,6 +40196,9 @@ msgstr ""
 msgid "When enabled, existing personal access tokens may be revoked. Leave blank for no limit."
 msgstr ""
 
+msgid "When enabled, job logs are collected by Datadog and displayed along with pipeline execution traces."
+msgstr ""
+
 msgid "When inactive, an external authentication provider must be used."
 msgstr ""
 

spec/lib/gitlab/data_builder/archive_trace_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::DataBuilder::ArchiveTrace do
+  let_it_be(:build) { create(:ci_build, :trace_artifact) }
+
+  describe '.build' do
+    let(:data) { described_class.build(build) }
+
+    it 'has correct attributes', :aggregate_failures do
+      expect(data[:object_kind]).to eq 'archive_trace'
+      expect(data[:trace_url]).to eq build.job_artifacts_trace.file.url
+      expect(data[:build_id]).to eq build.id
+      expect(data[:pipeline_id]).to eq build.pipeline_id
+      expect(data[:project]).to eq build.project.hook_attrs
+    end
+  end
+end

spec/models/integration_spec.rb

@@ -107,33 +107,25 @@ RSpec.describe Integration do
       end
     end
 
-    describe '.confidential_note_hooks' do
-      it 'includes integrations where confidential_note_events is true' do
-        create(:integration, active: true, confidential_note_events: true)
+    shared_examples 'hook scope' do |hook_type|
+      describe ".#{hook_type}_hooks" do
+        it "includes services where #{hook_type}_events is true" do
+          create(:integration, active: true, "#{hook_type}_events": true)
 
-        expect(described_class.confidential_note_hooks.count).to eq 1
-      end
+          expect(described_class.send("#{hook_type}_hooks").count).to eq 1
+        end
 
-      it 'excludes integrations where confidential_note_events is false' do
-        create(:integration, active: true, confidential_note_events: false)
+        it "excludes services where #{hook_type}_events is false" do
+          create(:integration, active: true, "#{hook_type}_events": false)
 
-        expect(described_class.confidential_note_hooks.count).to eq 0
+          expect(described_class.send("#{hook_type}_hooks").count).to eq 0
+        end
       end
     end
 
-    describe '.alert_hooks' do
-      it 'includes integrations where alert_events is true' do
-        create(:integration, active: true, alert_events: true)
-
-        expect(described_class.alert_hooks.count).to eq 1
-      end
-
-      it 'excludes integrations where alert_events is false' do
-        create(:integration, active: true, alert_events: false)
-
-        expect(described_class.alert_hooks.count).to eq 0
-      end
-    end
+    include_examples 'hook scope', 'confidential_note'
+    include_examples 'hook scope', 'alert'
+    include_examples 'hook scope', 'archive_trace'
   end
 
   describe '#operating?' do

spec/models/integrations/datadog_spec.rb

@@ -38,6 +38,11 @@ RSpec.describe Integrations::Datadog do
 
   let(:pipeline_data) { Gitlab::DataBuilder::Pipeline.build(pipeline) }
   let(:build_data) { Gitlab::DataBuilder::Build.build(build) }
+  let(:archive_trace_data) do
+    create(:ci_job_artifact, :trace, job: build)
+
+    Gitlab::DataBuilder::ArchiveTrace.build(build)
+  end
 
   it_behaves_like Integrations::HasWebHook do
     let(:integration) { instance }
@@ -100,6 +105,13 @@ RSpec.describe Integrations::Datadog do
     end
   end
 
+  describe '#help' do
+    subject { instance.help }
+
+    it { is_expected.to be_a(String) }
+    it { is_expected.not_to be_empty }
+  end
+
   describe '#hook_url' do
     subject { instance.hook_url }
 
@@ -161,10 +173,13 @@ RSpec.describe Integrations::Datadog do
     end
 
     before do
+      stub_feature_flags(datadog_integration_logs_collection: enable_logs_collection)
       stub_request(:post, expected_hook_url)
       saved_instance.execute(data)
     end
 
+    let(:enable_logs_collection) { true }
+
     context 'with pipeline data' do
       let(:data) { pipeline_data }
       let(:expected_headers) { { ::Gitlab::WebHooks::GITLAB_EVENT_HEADER => 'Pipeline Hook' } }
@@ -180,5 +195,19 @@ RSpec.describe Integrations::Datadog do
 
       it { expect(a_request(:post, expected_hook_url).with(headers: expected_headers, body: expected_body)).to have_been_made }
     end
+
+    context 'with archive trace data' do
+      let(:data) { archive_trace_data }
+      let(:expected_headers) { { ::Gitlab::WebHooks::GITLAB_EVENT_HEADER => 'Archive Trace Hook' } }
+      let(:expected_body) { data.to_json }
+
+      it { expect(a_request(:post, expected_hook_url).with(headers: expected_headers, body: expected_body)).to have_been_made }
+
+      context 'but feature flag disabled' do
+        let(:enable_logs_collection) { false }
+
+        it { expect(a_request(:post, expected_hook_url)).not_to have_been_made }
+      end
+    end
   end
 end

spec/requests/api/integrations_spec.rb

@@ -55,8 +55,10 @@ RSpec.describe API::Integrations do
           current_integration = project.integrations.first
           events = current_integration.event_names.empty? ? ["foo"].freeze : current_integration.event_names
           query_strings = []
-          events.each do |event|
-            query_strings << "#{event}=#{!current_integration[event]}"
+          events.map(&:to_sym).each do |event|
+            event_value = !current_integration[event]
+            query_strings << "#{event}=#{event_value}"
+            integration_attrs[event] = event_value if integration_attrs[event].present?
           end
           query_strings = query_strings.join('&')
 

spec/services/ci/archive_trace_service_spec.rb

@@ -15,6 +15,25 @@ RSpec.describe Ci::ArchiveTraceService, '#execute' do
       expect(job.trace_metadata.trace_artifact).to eq(job.job_artifacts_trace)
     end
 
+    context 'integration hooks' do
+      it do
+        stub_feature_flags(datadog_integration_logs_collection: [job.project])
+
+        expect(job.project).to receive(:execute_integrations) do |data, hook_type|
+          expect(data).to eq Gitlab::DataBuilder::ArchiveTrace.build(job)
+          expect(hook_type).to eq :archive_trace_hooks
+        end
+        expect { subject }.not_to raise_error
+      end
+
+      it 'with feature flag disabled' do
+        stub_feature_flags(datadog_integration_logs_collection: false)
+
+        expect(job.project).not_to receive(:execute_integrations)
+        expect { subject }.not_to raise_error
+      end
+    end
+
     context 'when trace is already archived' do
       let!(:job) { create(:ci_build, :success, :trace_artifact) }