Skip to content

Validate span.type and .subtype against shared spec#1030

Merged
mikker merged 14 commits intoelastic:masterfrom
mikker:shared-span-types
Jul 7, 2021
Merged

Validate span.type and .subtype against shared spec#1030
mikker merged 14 commits intoelastic:masterfrom
mikker:shared-span-types

Conversation

@mikker
Copy link
Copy Markdown
Contributor

@mikker mikker commented Jun 24, 2021

Closes #1018

  • Found out that we still sent types in the legacy joined form type.subtype.action. Turns out this is still Good Behaviour™
  • Instead of changing any of the metadata we currently report, this PR only checks against and adds to the spec. This is because we want to get a full picture of the status quo before we possibly start aligning between agents.

@ghost
Copy link
Copy Markdown

ghost commented Jun 24, 2021

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview preview

Expand to view the summary

Build stats

  • Start Time: 2021-07-06T18:27:30.639+0000

  • Duration: 32 min 34 sec

  • Commit: bbdc73c

Test stats 🧪

Test Results
Failed 0
Passed 45373
Skipped 84
Total 45457

Trends 🧪

Image of Build Times

Image of Tests

@mikker mikker marked this pull request as ready for review June 29, 2021 12:17
@mikker mikker requested review from SylvainJuge and estolfo June 29, 2021 12:17
Copy link
Copy Markdown
Member

@SylvainJuge SylvainJuge left a comment

Choose a reason for hiding this comment

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

Just a few minor questions/comments on my end for the shared JSON spec.

I start to feel that we could add another descriptive field to indicate which agent is using what, that will later help to align and find which ones are mostly used, and which ones are the exceptions & oddities, what do you think ?

@mikker
Copy link
Copy Markdown
Contributor Author

mikker commented Jun 29, 2021

I think marking agent langs in the spec is a good idea so long as we are in the process of aligning 👍

@SylvainJuge
Copy link
Copy Markdown
Member

I've just updated the original PR (elastic/apm#443) with __used_by field, not sure if the wording/layout of this is the best but at least we can use it.

@mikker mikker requested a review from SylvainJuge July 6, 2021 11:40
@SylvainJuge
Copy link
Copy Markdown
Member

There are only a few differences with the elastic/apm#443 PR, thus you can directly update it once this is merged without doing a PR on PR :-).

@mikker mikker merged commit 3cf3082 into elastic:master Jul 7, 2021
@mikker mikker deleted the shared-span-types branch July 7, 2021 07:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Implement span.type shared spec

3 participants