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

Change profiling HTTP request runtime and type fields #1166

Merged
merged 5 commits into from
Sep 9, 2020

Conversation

flodav
Copy link
Contributor

@flodav flodav commented Sep 4, 2020

We want the profiling analyzer to process Ruby profiles. Today the quickest way is to set the type field to auto while waiting for some heavy refactoring into the analyzer.

We allow ourself to do this change since the profiler is still in beta.

@flodav flodav requested a review from a team September 4, 2020 10:19
@flodav flodav changed the base branch from master to feature/profiling September 4, 2020 10:19
@flodav flodav requested review from delner and removed request for a team September 4, 2020 10:19
@flodav flodav force-pushed the fd/PROF-1952-set-type-to-auto branch from 8c814ec to b412c0a Compare September 4, 2020 10:33
@delner delner force-pushed the fd/PROF-1952-set-type-to-auto branch from b412c0a to 1049424 Compare September 8, 2020 19:59
@delner delner changed the title [PROF-1952] Set profiling request 'type' field to auto Change profiling HTTP request runtime and type fields Sep 8, 2020
@delner delner self-assigned this Sep 8, 2020
@delner delner added the core Involves Datadog core libraries label Sep 8, 2020
@delner
Copy link
Contributor

delner commented Sep 8, 2020

Made some updates to this:

  • Fixed tests for the types[0] = 'auto' change
  • Changed runtime tag to reflect language instead of interpreter (engine + platform)
  • Added runtime_engine and runtime_platform tags

These changes should address some needed things for the backend.

@delner delner force-pushed the fd/PROF-1952-set-type-to-auto branch from 1049424 to 4509319 Compare September 8, 2020 21:11
@delner delner force-pushed the fd/PROF-1952-set-type-to-auto branch from 4509319 to be71575 Compare September 9, 2020 00:32
Copy link
Contributor Author

@flodav flodav left a comment

Choose a reason for hiding this comment

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

LGTM but I can't approve my own PR. Maybe @jd you can take a look to review it ?

Copy link
Member

@marcotc marcotc left a comment

Choose a reason for hiding this comment

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

👍

@@ -27,7 +28,8 @@ def initialize(*args)
self.version = version || Datadog.configuration.version
self.host = host || Datadog::Runtime::Socket.hostname
self.language = language || Datadog::Runtime::Identity.lang
self.runtime = runtime || Datadog::Runtime::Identity.lang_interpreter
self.runtime_engine = runtime_engine || Datadog::Runtime::Identity.lang_engine
self.runtime_platform = runtime_platform || Datadog::Runtime::Identity.lang_platform
Copy link
Member

Choose a reason for hiding this comment

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

We might want to report these in the tracer payload as well. Nothing to do in this PR, just a reminder for our future selves.

Copy link
Contributor

Choose a reason for hiding this comment

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

Good idea. In the tracer payload I think we send them as "interpreter" which is both terms combined.

@delner delner merged commit 349f89a into feature/profiling Sep 9, 2020
@delner delner deleted the fd/PROF-1952-set-type-to-auto branch September 9, 2020 15:17
ivoanjo added a commit that referenced this pull request Jan 5, 2022
This behavior was removed in #1166 but a test was left behind that no
longer was relevant.
ivoanjo added a commit that referenced this pull request Jan 6, 2022
This behavior was removed in #1166 but a test was left behind that no
longer was relevant.
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.

3 participants