[PROF-4535] Switch profiling to use intake 1.3 format #1820
Conversation
This behavior was removed in #1166 but a test was left behind that no longer was relevant.
These tests don't actually need an agent (they open their own web server), and I can't think of a reason not to run them by default, like we do for other similar tests.
By avoiding such constants we: 1. Make our tests more independent of the actual code being tested -- we can freely refactor the internal structure and where we store the constants, as long as the result is the same. 2. Actually validate the expected payload in the tests. E.g., if a field is renamed, that change MUST be reflected in the tests and thus it becomes very obvious that such a change was made.
Although not stated explicitly, the Ruby profiler was previously using the 1.2 intake format. The 1.3 format (lightly documented [here](https://github.com/DataDog/profiling-backend/blob/prod/README.md#v3-intake-format-used-by-go-net-native) [Datadog-internal link, apologies]) shuffles around the fields a bit: * `recording-start` => `start` * `recording-end` => `end` * `data[0]` + `types[0]` => `data[somefilename]` * `runtime` => `family` * `format` is removed * `version` is added This change is not observable to customers; but is a requirement to submitting extra files along with profiles, as we plan to do in #1813.
This left an open socket behind, which conflicted with other specs.
ivoanjo
force-pushed
the
ivoanjo/prof-4535-profiling-use-intake-1.3-format
branch
from
778e031 to
3639187
Compare
Codecov Report
@@ Coverage Diff @@
## master #1820 +/- ##
==========================================
- Coverage 98.21% 98.21% -0.01%
==========================================
Files 931 931
Lines 44920 44909 -11
==========================================
- Hits 44120 44109 -11
Misses 800 800
Continue to review full report at Codecov.
|
| FORM_FIELD_DATA = 'data[0]'.freeze | ||
| FORM_FIELD_FORMAT = 'format'.freeze | ||
| FORM_FIELD_FORMAT_PPROF = 'pprof'.freeze | ||
| FORM_FIELD_RECORDING_END = 'recording-end'.freeze | ||
| FORM_FIELD_RECORDING_START = 'recording-start'.freeze | ||
| FORM_FIELD_RUNTIME = 'runtime'.freeze | ||
| FORM_FIELD_RUNTIME_ID = 'runtime-id'.freeze |
There was a problem hiding this comment.
Just to confirm, we don't need to support version 1.2 anymore at all?
I ask because, for tracing when we introduce a new transport version, we normally keep the old one as the user's Datadog agent might not be up to date with the latest transport version yet.
There was a problem hiding this comment.
That's a great question :)
We don't need to support the old version at all because for profiling, there's no format-specific logic in the agent. The agent just acts as a rather dumb proxy -- receives stuff from the library and then repeats it back to the profiling backend.
So we are free to make these changes.
Also, there aren't yet any plans to sunset the old format, so older versions of the profiler will continue to work (but we always recommend being on the latest!).
Although not stated explicitly, the Ruby profiler was previously using the 1.2 intake format.
The 1.3 format (lightly documented here [Datadog-internal link, apologies]) shuffles around the fields a bit:
recording-start=>startrecording-end=>enddata[0]+types[0]=>data[somefilename]runtime=>familyformatis removedversionis addedThis change is not observable to customers; but is a requirement to submitting extra files along with profiles, as we plan to do in #1813.
I also included a few cleanups to the specs; I suggest reviewing this PR commit-by-commit.
Since this PR needs some profiling-backend specific knowledge to validate (aka that I'm using the v3 format correctly), I'll also find someone from the Profiling team to do a pass on those bits.
P.s.: CI is broken due to unrelated reasons. Will be tackled separately.