[Debugger] Ensure all probes have a version property

[watson]

Motivation

All probes sent via Remote Config should have a root version property. For some reason, one was not present in our mocked probe configs located in tests/debugger/probes. I'm not sure why this hasn't resulted in system test errors previously since the diagnostics schema expects that a version is reported. However, without this, at least the Node.js tracer, would set version: undefined in the object before it was serialized to JSON, which would completely remove the version property from the JSON output.

Maybe they other tracers fall back to 0 if there's no version property in the probe config?

Changes

Hardcode all mocked probe configs to contain version: 0.

Workflow

1. ⚠️ Create your PR as draft ⚠️
2. Work on you PR until the CI passes (if something not related to your task is failing, you can ignore it)
3. Mark it as ready for review
   • Test logic is modified? -> Get a review from RFC owner. We're working on refining the codeowners file quickly.
   • Framework is modified, or non obvious usage of it -> get a review from R&P team

🚀 Once your PR is reviewed, you can merge it!

🛟 #apm-shared-testing 🛟

Reviewer checklist

• If PR title starts with [<language>], double-check that only <language> is impacted by the change
• No system-tests internal is modified. Otherwise, I have the approval from R&P team
• CI is green, or failing jobs are not related to this change (and you are 100% sure about this statement)
• A docker base image is modified?
  • the relevant build-XXX-image label is present
• A scenario is added (or removed)?
  • Get a review from R&P team

[watson]

• [Debugger] Scope the expected Python schema errors to just Python #3583
• [Debugger] Re-enable Node.js tests that should now be working #3582
• [Debugger] Ensure all probes have a version property #3581 👈 (View in Graphite)
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[robertomonteromiguel]

From the system-tests core point of view it is correct. But you would need the approval of other teams.

[p-datadog]

This PR seems reasonable since it only adds a field to the input (which I believe Ruby implementation will ignore), but I will test locally before adding my checkmark.

[shurivich]

We have a method of enriching all probes with required information.

system-tests/tests/debugger/utils.py

Line 76 in 02c78ef

def _enrich_probes(probes):

I suggest to add the code:
probe["version"] = 0

after the line

system-tests/tests/debugger/utils.py

Line 92 in 02c78ef

probe["language"] = language

instead of modifying all json probe files.

[watson]

What's the reason we don't want to update the json at the source?

[shurivich]

What's the reason we don't want to update the json at the source?

Single responsibility - updates are handled in one place, ensuring clarity.
Less changes.

But maybe it's just a matter of code taste, so it's up to you to decide.
Wanted to make sure that you're aware of this option, too.

[watson]

@shurivich I think it makes a lot of sense to have a way to dynamically update the JSON to suite the language being instrumented and other non-fixed properties - exactly like you've done in tests/debugger/utils.py - otherwise we'd end up with a lot of duplication 👍

However, the version property is currently static - just like the id property - and doesn't depend on the tracer (this isn't the version of the tracer, but the "probe version" and is tightly connected to the id).

So if you don't mind, I'll just leave it as is - at least for now. I think that makes it easier to understand the schema when just viewing the JSON file. Anyways, we can always refactor it if we want to make to the probe version dynamic ☺️

[cbeauchesne]

Sorry, the script that computes which scenario to run failed, so this PR hasn't be tested 😢

I've reverted it, and fixed the script.

To correct this PR, you must very probably update the hash, as the content you've sent has changed, I can help you to do so.