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

fix: make sure at least one of the service target fields is sent #1277

Merged
merged 2 commits into from
Jul 29, 2022

Conversation

kruskall
Copy link
Member

According to the spec:

'at least one of those two fields is required to be provided and not empty'

While both are optional, the only one with an explicit clause that
allows empty values is 'type'.
To better reflect that, 'type' is always included (even if empty) and
'name' is omitted if empty.

See https://github.com/elastic/apm/blob/main/specs/agents/tracing-spans-service-target.md

Additionally, unused json tags are removed from internal fields.

According to the spec:

'at least one of those two fields is required to be provided and not empty'

While both are optional, the only one with an explicit clause that
allows empty values is 'type'.
To better reflect that, 'type' is always included (even if empty) and
'name' is omitted if empty.

See https://github.com/elastic/apm/blob/main/specs/agents/tracing-spans-service-target.md

Additionally, unused json tags are removed from internal fields.
@apmmachine
Copy link
Contributor

apmmachine commented Jul 29, 2022

💚 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: 2022-07-29T17:10:17.301+0000

  • Duration: 73 min 54 sec

Test stats 🧪

Test Results
Failed 0
Passed 7475
Skipped 174
Total 7649

🤖 GitHub comments

To re-run your PR in the CI, just comment with:

  • /test : Re-trigger the build.

  • run benchmark tests : Run the benchmark test.

  • run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

@apmmachine
Copy link
Contributor

apmmachine commented Jul 29, 2022

🌐 Coverage report

Name Metrics % (covered/total) Diff
Packages 100.0% (59/59) 💚
Files 99.346% (152/153)
Classes 95.714% (335/350)
Methods 90.019% (956/1062)
Lines 81.916% (11098/13548)
Conditionals 100.0% (0/0) 💚

@kruskall kruskall merged commit f3747db into elastic:main Jul 29, 2022
@kruskall kruskall deleted the fix/service-target-empty branch July 29, 2022 18:41
v1v added a commit to v1v/apm-agent-go that referenced this pull request Sep 1, 2022
…hmark-reporting

* upstream/main: (25 commits)
  docs: update correct env flag for loglevel (elastic#1299)
  fix: readd deprecated span_frames_min_duration option as fallback for older configuration (elastic#1297)
  feat: rename span_frames_min_duration to span_stack_trace_min_duration (elastic#1285)
  test: verify Ubuntu cgroup line parsing for container ID (elastic#1293)
  tracer: Parse global labels per tracer (elastic#1290)
  feat: update sns span.name to reflect the current spec (elastic#1286)
  fix: expand k8s pod discovery regex (elastic#1288)
  test: add testcase for sqs delete_batch operation (elastic#1283)
  docs: document ELASTIC_APM_SERVER_CA_CERT_FILE (elastic#1289)
  fix: reformat code with go 1.19 to fix ci failure  (elastic#1284)
  feat: add service target fields support to elasticsearch module (elastic#1281)
  fix: use the correct destination resource and name for azure queue (elastic#1282)
  feat: add service target fields support to azure module (elastic#1280)
  feat: add service target fields support to aws module (elastic#1278)
  feat: add service target fields support to sql module (elastic#1279)
  synchronize json schema specs (elastic#1260)
  fix: make sure at least one of the service target fields is sent (elastic#1277)
  docs: add link to release-notes-2.x (elastic#1271)
  feat: add service target fields (elastic#1274)
  perf: skip tracestate regex validation for es vendor key (elastic#1275)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants