NH-52515 Remove cached custom name at calculate_custom #187
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
swi-jared
requested changes
Aug 2, 2023
Added in 1d7445f. Going to keep going on this PR to add tests for other processor functions! |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
Thanks @swi-jared ! Lots more unit tests now. Please could you have a look when you have a moment. |
| mocker.Mock(), | ||
| mocker.Mock(), | ||
| ) | ||
| result = processor.calculate_transaction_names(mock_span) |
Check notice
Code scanning / CodeQL
Unused local variable
| mocker.Mock(), | ||
| mocker.Mock(), | ||
| ) | ||
| result = processor.calculate_transaction_names(mock_span) |
Check notice
Code scanning / CodeQL
Unused local variable
| mocker.Mock(), | ||
| mocker.Mock(), | ||
| ) | ||
| result = processor.calculate_transaction_names(mock_span) |
Check notice
Code scanning / CodeQL
Unused local variable
swi-jared
approved these changes
Aug 3, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
EDIT: Also adds several unit tests for inbound metrics processor.
Adds removal of any stored custom transaction name at the end of
calculate_custom_transaction_name. It will only be re-added to the manager if span is sampled so that exporter will have access to the name, before removing name again after reporting. This is to stop leaking in the case where span is not sampled but custom transaction name is set (used for inbound metrics reporting). Big thanks to @swi-jared for seeing this!Step-wise:
set_transaction_name("foo"). Call is successful and manager stores"<trace_id>-<span_id>": "foo".<span_id>andon_endof processors called for all recorded spans, sampled or not (already in place!)calculate_transaction_namesfor inbound metrics is called. Helpercalculate_custom_transaction_namegets"<trace_id>-<span_id>": "foo"from manager.calculate_custom_transaction_namedeletes"<trace_id>-<span_id>": "foo"from manager.on_end."<trace_id>-<span_id>": "foo"in manager. This can be a re-cache of custom name or new cache of 'regular' transaction name.a. Exporter gets
"<trace_id>-<span_id>": "foo"from manager.b. Exporter deletes
"<trace_id>-<span_id>": "foo"from manager.Test trace from Django A testbed app's "reentry" route that requests its own route and also uses API set_transaction_name --
sw.transactioniscustom-name-django-a-rootat the root span, and iscustom-name-reentryat theSERVER:GET another/span. Just a quick smoke test to make sure custom txn naming still works: https://my.na-01.cloud.solarwinds.com/140638900734749696/traces/E51FD08B882DA4677B26254C08F5701A/E5C99F94B8B0D44A/details/breakdown?perspective=requests&serviceId=e-1540126275982954496I made a second request a few minutes later with APM Python hardcoded to always decide
RECORD_ONLY. This is to make sure we get metrics blips for the same custom names even if no trace exported: