Skip to content

Add public API to retrieve trace parent header.#956

Merged
basepi merged 6 commits into
elastic:masterfrom
diegosperes:issue-954
Oct 30, 2020
Merged

Add public API to retrieve trace parent header.#956
basepi merged 6 commits into
elastic:masterfrom
diegosperes:issue-954

Conversation

@diegosperes

@diegosperes diegosperes commented Oct 27, 2020 •
edited
Loading

Copy link
Copy Markdown
Contributor

What does this pull request do?

Add a new public API to easily return the trace parent header of the current transaction, currently it's necessary to dig into the project and understand how it works.

Code necessary to get the current trace parent header without the new function.

from elasticapm.traces import execution_context

transaction = execution_context.get_transaction().
trace_parent_header = transaction .trace_parent.to_string() if transaction and transaction.trace_parent else None

Related issues

#954

@cla-checker-service

cla-checker-service Bot commented Oct 27, 2020 •
edited
Loading

Copy link
Copy Markdown

💚 CLA has been signed

@ghost

ghost commented Oct 27, 2020 •
edited by ghost
Loading

Copy link
Copy Markdown

💚 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

Expand to view the summary

Build stats

  • Build Cause: [Started by user Benjamin Wohlwend, Replayed #7]

  • Start Time: 2020-10-28T11:50:24.612+0000

  • Duration: 22 min 36 sec

Test stats 🧪

Test Results
Failed 0
Passed 11238
Skipped 8409
Total 19647

Steps errors 1

Expand to view the steps failures

  • Name: Shell Script

    • Description: ./tests/scripts/docker/run_tests.sh pypy-3 django-1.11

    • Duration: 3 min 58 sec

    • Start Time: 2020-10-28T11:55:51.496+0000

    • log

@diegosperes

diegosperes commented Oct 27, 2020 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Couldn't find which place I should add the unit test, does someone has any suggestion? I think there isn't a test case for public API.

@beniwohli

beniwohli commented Oct 28, 2020

Copy link
Copy Markdown
Contributor

Hey @diegosperes. This looks like a nice addition to the public API! You're right, the testing for public APIs isn't very extensive, but there are a few here that might serve as inspiration:

apm-agent-python/tests/instrumentation/transactions_store_tests.py

Lines 363 to 442 in 65e8b7c

def test_set_transaction_name(elasticapm_client):
elasticapm_client.begin_transaction("test")
elasticapm_client.end_transaction("test_name", 200)
elasticapm_client.begin_transaction("test")
elasticapm.set_transaction_name("another_name")
elasticapm_client.end_transaction("test_name", 200)
transactions = elasticapm_client.events[TRANSACTION]
assert transactions[0]["name"] == "test_name"
assert transactions[1]["name"] == "another_name"
def test_set_transaction_custom_data(elasticapm_client):
elasticapm_client.begin_transaction("test")
elasticapm.set_custom_context({"foo": "bar"})
elasticapm_client.end_transaction("foo", 200)
transactions = elasticapm_client.events[TRANSACTION]
assert transactions[0]["context"]["custom"] == {"foo": "bar"}
def test_set_transaction_custom_data_merge(elasticapm_client):
elasticapm_client.begin_transaction("test")
elasticapm.set_custom_context({"foo": "bar", "bar": "baz"})
elasticapm.set_custom_context({"bar": "bie", "boo": "biz"})
elasticapm_client.end_transaction("foo", 200)
transactions = elasticapm_client.events[TRANSACTION]
assert transactions[0]["context"]["custom"] == {"foo": "bar", "bar": "bie", "boo": "biz"}
def test_set_user_context(elasticapm_client):
elasticapm_client.begin_transaction("test")
elasticapm.set_user_context(username="foo", email="[email protected]", user_id=42)
elasticapm_client.end_transaction("foo", 200)
transactions = elasticapm_client.events[TRANSACTION]
assert transactions[0]["context"]["user"] == {"username": "foo", "email": "[email protected]", "id": 42}
def test_set_user_context_merge(elasticapm_client):
elasticapm_client.begin_transaction("test")
elasticapm.set_user_context(username="foo", email="[email protected]")
elasticapm.set_user_context(email="[email protected]", user_id=42)
elasticapm_client.end_transaction("foo", 200)
transactions = elasticapm_client.events[TRANSACTION]
assert transactions[0]["context"]["user"] == {"username": "foo", "email": "[email protected]", "id": 42}
def test_callable_context_ignored_when_not_sampled(elasticapm_client):
callable_data = mock.Mock()
callable_data.return_value = {"a": "b"}
transaction = elasticapm_client.begin_transaction("test")
transaction.is_sampled = False
elasticapm.set_context({"c": "d"})
elasticapm.set_context(callable_data)
elasticapm_client.end_transaction("test", "OK")
transaction = elasticapm_client.events[TRANSACTION][0]
assert callable_data.call_count == 0
assert "context" not in transaction
def test_dedot_context_keys(elasticapm_client):
elasticapm_client.begin_transaction("test")
elasticapm.set_context({"d.o.t": "d_o_t", "s*t*a*r": "s_t_a_r", "q*u*o*t*e": "q_u_o_t_e"})
elasticapm_client.end_transaction("foo", 200)
transaction = elasticapm_client.events[TRANSACTION][0]
assert transaction["context"]["custom"] == {"s_t_a_r": "s_t_a_r", "q_u_o_t_e": "q_u_o_t_e", "d_o_t": "d_o_t"}

It would also be great if you could add an entry in the traceparent docs here: https://github.com/elastic/apm-agent-python/blob/master/docs/api.asciidoc#traceparent.

@diegosperes

diegosperes commented Oct 28, 2020 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

@beniwohli thanks for guide me, I added a few missing test to the public API also a new entry for this function in the traceparent docs.

@ghost

ghost commented Oct 28, 2020 •
edited by ghost
Loading

Copy link
Copy Markdown

💚 Flaky test report

Tests succeeded.

Expand to view the summary

Test stats 🧪

Test Results
Failed 0
Passed 11238
Skipped 8409
Total 19647

beniwohli
beniwohli approved these changes Oct 28, 2020
View reviewed changes

@beniwohli beniwohli left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is great work, thanks a lot!

@basepi ok with merging this?

@basepi basepi merged commit 2a6f779 into elastic:master Oct 30, 2020
@basepi basepi mentioned this pull request Oct 30, 2020
Closed
@bmorelli25 bmorelli25 mentioned this pull request Oct 30, 2020
Merged
beniwohli pushed a commit to beniwohli/apm-agent-python that referenced this pull request Sep 14, 2021
@diegosperes @beniwohli
Add public API to retrieve trace parent header. (elastic#956)
fb975fa 
* Add public API to retrieve trace parent header.

* Exposing get_trace_parent_header function

* Add missing tests for public api.

* Add get_trace_parent_header in the traceparent docs.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@basepi basepi basepi approved these changes
+1 more reviewer
@beniwohli beniwohli beniwohli approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@diegosperes @beniwohli @basepi