Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add remote configuration client #2689

Merged
merged 7 commits into from
Mar 21, 2023
Merged

Conversation

lloeki
Copy link
Contributor

@lloeki lloeki commented Mar 13, 2023

What does this PR do?

Implement the remote configuration client class.

Motivation

Remote configuration

Additional Notes

Barebones implementation, some questions remain to be solved:

  • there is AppSec-specific code in there (products and capabilities declaration)
  • the callback system is missing because the repository does not yet report changes
  • caching is not handled

There is an issue where the backend appears not to report any client_configs nor target_files, and targets may have targets but they may not correspond and/or include to the active client id.

In such gases, calling client.sync multiple times ends up getting the configuration. At this stage it is unknown whether this is expected API endpoint behaviour (e.g a delay in creating entries for a new client id, a "data lag" inside the agent...)

How to test the change?

  require 'ddtrace'

  Datadog.configure do |c|
    c.agent.host = '192.168.135.133'
    #c.tracing.enabled = true
    #c.diagnostics.debug = true
  end

  transport_options = {
    agent_settings: Datadog::Core::Configuration::AgentSettingsResolver.call(
      Datadog.configuration
    )
  }

  require 'datadog/core/transport/http'

  transport_v7 = Datadog::Core::Transport::HTTP.v7(**transport_options.dup)

  require 'datadog/core/remote/client'

  client = Datadog::Core::Remote::Client.new(transport_v7)
  repository = client.instance_eval { @repository }

  client.sync
  
  repository.contents

@github-actions github-actions bot added the core Involves Datadog core libraries label Mar 13, 2023
@lloeki
Copy link
Contributor Author

lloeki commented Mar 14, 2023

Pending:

  • specs
  • RBS typing

@lloeki
Copy link
Contributor Author

lloeki commented Mar 16, 2023

  • Rebased using git rebase --onto origin/add-remote-config-repository 414a6260c add-remote-config-client
  • Fixed usage of ContentList#find which was renamed to #find_content

@lloeki
Copy link
Contributor Author

lloeki commented Mar 16, 2023

Rebased using git rebase --onto add-remote-config-repository c9ef278c6^ add-remote-config-client

Base automatically changed from add-remote-config-repository to master March 17, 2023 07:59
lloeki and others added 3 commits March 17, 2023 09:03
Barebones implementation, some questions remain to be solved.
Remote configuration spec says that an empty Hash in a response means
NOOP, not "unset everything".
@GustavoCaso
Copy link
Member

Rebased using git rebase master

@GustavoCaso GustavoCaso marked this pull request as ready for review March 17, 2023 13:08
@GustavoCaso GustavoCaso requested a review from a team March 17, 2023 13:08
@codecov-commenter
Copy link

Codecov Report

Merging #2689 (5220533) into master (a692552) will increase coverage by 0.00%.
The diff coverage is 99.51%.

@@           Coverage Diff            @@
##           master    #2689    +/-   ##
========================================
  Coverage   98.03%   98.03%            
========================================
  Files        1197     1200     +3     
  Lines       65714    65915   +201     
  Branches     2926     2934     +8     
========================================
+ Hits        64420    64622   +202     
+ Misses       1294     1293     -1     
Impacted Files Coverage Δ
lib/datadog/core/transport/http/config.rb 84.25% <90.00%> (-0.31%) ⬇️
lib/datadog/core/remote/client.rb 100.00% <100.00%> (ø)
lib/datadog/core/remote/configuration.rb 100.00% <100.00%> (ø)
lib/datadog/core/transport/config.rb 96.29% <100.00%> (+0.29%) ⬆️
spec/datadog/core/remote/client_spec.rb 100.00% <100.00%> (ø)

... and 5 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more


private

def payload: () -> ::Hash[Symbol, untyped]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess we could improve this untyped given that we expect only a specific set of types here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good. We can tackle that on a follow up PR

@GustavoCaso GustavoCaso merged commit 5edffaa into master Mar 21, 2023
@GustavoCaso GustavoCaso deleted the add-remote-config-client branch March 21, 2023 09:24
@github-actions github-actions bot added this to the 1.11.0 milestone Mar 21, 2023
@lloeki lloeki modified the milestones: 1.11.0, 1.11.0.beta1 Apr 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Involves Datadog core libraries
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants