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 sourcemap logging #4132

Merged
merged 3 commits into from
Sep 3, 2020
Merged

Change sourcemap logging #4132

merged 3 commits into from
Sep 3, 2020

Conversation

simitt
Copy link
Contributor

@simitt simitt commented Sep 2, 2020

fixes #4119

Motivation/summary

Remove unnecessary log line for source mapping and change potentially interesting log line from warning to debug level.

Checklist

- [ ] I have signed the Contributor License Agreement.
- [ ] I have updated CHANGELOG.asciidoc

I have considered changes for:
- [ ] documentation

How to test these changes

Start APM Server with source mapping enabled

  • send request to RUM endpoint without service.name, check that nothing is logged.
  • send request to RUM endpoint for which no sourcemap exists, or ensure the connection to ES is failing, check that nothing is logged.

Change log level to debug and repeat sending a request for which no sourcemap can be retrieved - a message should be logged

Related issues

#4119

Remove unnecessary log line and change other from warning to debug level.

fixes elastic#4119
@apmmachine
Copy link
Contributor

apmmachine commented Sep 2, 2020

💚 Build Succeeded

Pipeline View Test View Changes Artifacts preview

Expand to view the summary

Build stats

  • Build Cause: [Pull request #4132 updated]

  • Start Time: 2020-09-03T05:57:48.301+0000

  • Duration: 50 min 4 sec

Test stats 🧪

Test Results
Failed 0
Passed 3255
Skipped 142
Total 3397

Steps errors

Expand to view the steps failures

  • Name: Compress

    • Description: tar --exclude=coverage-files.tgz -czf coverage-files.tgz coverage

    • Duration: 0 min 0 sec

    • Start Time: 2020-09-03T06:14:34.815+0000

    • log

  • Name: Compress

    • Description: tar --exclude=system-tests-linux-files.tgz -czf system-tests-linux-files.tgz system-tests

    • Duration: 0 min 0 sec

    • Start Time: 2020-09-03T06:28:25.194+0000

    • log

  • Name: Test Sync

    • Description: ./script/jenkins/sync.sh

    • Duration: 3 min 59 sec

    • Start Time: 2020-09-03T06:09:13.354+0000

    • log

@simitt simitt merged commit caf5c1f into elastic:master Sep 3, 2020
simitt added a commit to simitt/apm-server that referenced this pull request Sep 3, 2020
Remove unnecessary log line and change other from warning to debug level.

fixes elastic#4119
simitt added a commit that referenced this pull request Sep 4, 2020
Remove unnecessary log line and change other from warning to debug level.

fixes #4119
@simitt simitt deleted the sourcemap-logging branch October 6, 2020 10:12
@jalvz
Copy link
Contributor

jalvz commented Oct 14, 2020

@simitt Im confused by the test instructions, service.name is required in the payload, isnt it?

@simitt
Copy link
Contributor Author

simitt commented Oct 14, 2020

There was a bug in the intake validation, so that service.name was allowed to be empty (see #4141); since it is fixed now, the test instructions are outdated; I suggest you test with empty service.version.

@jalvz jalvz self-assigned this Oct 14, 2020
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.

Source mapping should not log warnings when service.version is unspecified
4 participants