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

Implement telemetry in tracer #2153

Merged
merged 15 commits into from
Aug 4, 2022
Merged

Implement telemetry in tracer #2153

merged 15 commits into from
Aug 4, 2022

Conversation

jennchenn
Copy link
Member

@jennchenn jennchenn commented Jul 13, 2022

What does this PR do?

This PR is a culmination of all the telemetry work. When telemetry is enabled, app-started, app-integrations-change, app-heartbeat and app-closing events should be sent to the telemetry API when applicable.

Motivation

Changes have been reviewed incrementally and merged into this feature branch; this PR just merges the feature branch.

Additional Notes

How to test the change?

Unit tests were added incrementally.

@jennchenn jennchenn requested a review from a team July 13, 2022 17:47
@jennchenn jennchenn added the feature Involves a product feature label Jul 13, 2022
@jennchenn jennchenn self-assigned this Jul 13, 2022
Copy link
Contributor

@delner delner left a comment

Choose a reason for hiding this comment

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

I think this should be good to go!

Just make sure CI is passing first; I think Rubocop updated and broke it. You may need to rebase this PR after master is fixed.

* Define classes relevant to app_started event

* Add more tests and base classes

* Use kwargs and invert naming

* Format multi-line blocks and sort parameters

* Add validation and unit tests

* Add documentaiton for param types and description

* fixup! Add validation and unit tests

* Condense namespace to Telemetry::V1::

* fixup! Condense namespace to Telemetry::V1::

* Remove type checking
* Collect loaded integrations and dependencies

* fixup! Collect loaded integrations and dependencies

* Collect system info

* Use stdlib etc

* Add top-level request method

* Add test cases for payload + fixups

* fixup! Add test cases for payload + fixups

* Fix sorbet errors

* Refactor collector module

* fixup! Refactor collector module

* Sort initialization args alphabetically

* Get configuration values from Datadog.configuration

* Use platform util for host information

* Collect only boolean configurations

* Reorganize collector module

* Store patch result when patch is called

* Capture patch errors for telemetry

* fixup! Capture patch errors for telemetry

* Update config variables based on standardized spec

* Implement configuration.to_hash to collect config variables

* Add dig function to configuration base

* fixup! Add dig function to configuration base
* Create Event class to instantiate TelemetryRequest

* Remove request_type, seq_id from Event init

* Run linter

* Remove validation tests

* Update request name

* Pass in seq_id as argument

* Remove api_version param from init
* Add telemetry enabled env flag

* Set DD_INSTRUMENTATION_TELEMETRY_ENABLED false in ci

* fixup! Set DD_INSTRUMENTATION_TELEMETRY_ENABLED false in ci

* Update GettingStarted doc

* Trigger CI

* Disable telemetry in test-head workflow
* Setup basic http post request methods

* Create env class

* Lint files

* Add basic transport tests

* Remove references to agentless

* Update transport tests

* Format files

* Use agent settings resolver to get agent url

* Rename file to http/transport

* Remove self.build method from net.rb

* Include Kernel to fix typechecking issue
* Define to_h functions for telemetry objects

* Add tests for to_h function

* fixup! Add tests for to_h function

* Remove appsec and profiler classes

* Change test dummy runtime session ID values
* Remove Configuration class

* Change products to always send appsec/profiler version
* Add module to emit telemetry event

* Handle seq_id autoincrement in emitter

* Fix emitter tests

* Add configuration tests

* Disable telemetry in CircleCI jobs

* Add test to integration apps

* fixup! Add test to integration apps

* Move app started call to components

* Add tests for emitter

* Add tests for client

* Add tests for components

* Modify integration tests

* fixup! Modify integration tests

* Use accessor for telemetry in integration tests
* Rename AppStarted to AppEvent

* Add app-integrations-change event

* Pass old components to Components#initialize

* Fix telemetry client tests

* Add common patcher methods to rack patcher
* Initialize worker to send heartbeat requests

* Add tests

* Change integration tests

* Wrap worker.stop and worker.join in if statement
* Change error logs to debug level

* Capture 404 errors and disable telemetry

* Set log to info to debug ci error

* Return connection errors as InternalErrorResponse

* fixup! Return connection errors as InternalErrorResponse
* Add reset! method to sequence util

* Create numeric sequence util

* Use sequence in emitter class

* fixup! Use sequence in emitter class

* Call telemetry started! separately

* Remove previous_components from components init

* Separate emit closing and shutdown

* Reset emitter sequence on fork

* fixup! Separate emit closing and shutdown

* Add fork policy to heartbeat worker

* Add mutex to sequence class

* Prevent emitting events if in forked process

* Use symbol for request type

* Move call to emit app-started to configuration

* Move call to emit integrations-change to extensions

* Add tests for forked? check in client

* Remove re-enable function in client

* Remove mutex from sequence numeric

* Use sequence class rather than sequence numeric
* Change telemetry imports to use require_relative

* Correct method argument line break linting issues
@jennchenn jennchenn merged commit 3c42872 into master Aug 4, 2022
@jennchenn jennchenn deleted the jenn/telemetry branch August 4, 2022 20:54
@github-actions github-actions bot added this to the 1.4.0 milestone Aug 4, 2022
settings :telemetry do
# Enable telemetry collection. This allows telemetry events to be emitted to the telemetry API.
#
# @default `DD_INSTRUMENTATION_TELEMETRY_ENABLED` environment variable, otherwise `true`
Copy link
Contributor

Choose a reason for hiding this comment

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

The changelog says that "Implement telemetry, disable by default", but this seems like telemetry is enabled by default?

Copy link
Member

Choose a reason for hiding this comment

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

Hey @gingerlime that's actually correct -- we initially merged it enabled (as you see here) but shortly before release we disabled it in #2233

Hope this helps :)

Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you. The changelog only links to the original PR so I didn't think to check. Perhaps you want to update it? in any case, it's good to know that it's disabled by default!

Copy link
Member

Choose a reason for hiding this comment

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

Ahhh right, oops, you're absolutely right and I've just updated the changelog with the link to the second PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you @ivoanjo !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Involves a product feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants