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(botocore): exclude params.Records from tags #2204

Merged
merged 6 commits into from
Mar 23, 2021

Conversation

P403n1x87
Copy link
Contributor

@P403n1x87 P403n1x87 commented Mar 22, 2021

Description

The params.Records argument could be big and therefore generate big trace payloads when added to the span tags. This change marks this argument for exclusion.

Checklist

  • Added to the correct milestone.
  • Tests provided or description of manual testing performed is included in the code or PR.
  • Library documentation is updated.
  • Corp site documentation is updated (link to the PR).

@P403n1x87 P403n1x87 added this to the 0.48.0 milestone Mar 22, 2021
@P403n1x87 P403n1x87 requested a review from a team as a code owner March 22, 2021 18:52
jd
jd previously approved these changes Mar 22, 2021
The params.Records argument could be big and therefore generate big
trace payloads when added to the span tags. This change marks this
argument for exclusion.
Copy link
Member

@brettlangdon brettlangdon left a comment

Choose a reason for hiding this comment

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

pending the suggested change to the release note 👍🏻

…yaml

Co-authored-by: Julien Danjou <julien@danjou.info>
@codecov-io
Copy link

Codecov Report

Merging #2204 (209852d) into master (d76ea73) will decrease coverage by 0.00%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2204      +/-   ##
==========================================
- Coverage   91.60%   91.59%   -0.01%     
==========================================
  Files         568      568              
  Lines       40767    40767              
==========================================
- Hits        37343    37342       -1     
- Misses       3424     3425       +1     
Impacted Files Coverage Δ
ddtrace/ext/aws.py 90.47% <ø> (ø)
tests/contrib/flask/test_blueprint.py 97.22% <0.00%> (-2.78%) ⬇️
ddtrace/profiling/exporter/http.py 98.91% <0.00%> (-1.09%) ⬇️
tests/contrib/django/django_app/settings.py 100.00% <0.00%> (+11.76%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d76ea73...209852d. Read the comment docs.

@mergify mergify bot merged commit 66eab83 into DataDog:master Mar 23, 2021
P403n1x87 added a commit to P403n1x87/dd-trace-py that referenced this pull request Mar 25, 2021
This change adds a test case for DataDog#2204 to ensure that the params.Records
argument is excluded from the span tags.
mergify bot added a commit that referenced this pull request Mar 25, 2021
This change adds a test case for #2204 to ensure that the params.Records
argument is excluded from the span tags.

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
@P403n1x87 P403n1x87 deleted the botocore-exclude-big-arg branch March 29, 2021 09:13
Kyle-Verhoog added a commit that referenced this pull request Apr 1, 2021
* fix typo in filter debug line (#2213)

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

* feat: support space separated DD_TAGS value (#2196)

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

* Add sync mode to AgentWriter (#2206)

* feat(agent): support aws lambda extension

* chore: fix flake8 issue with test

* chore: add release note for datadog lambda extension

* chore: fix import order

* chore: fix flake8 issue

* [serverless] fix broken test

* remove unused import

* separate out sync writer changes

* add integration tests for sync writer

Co-authored-by: DarcyRaynerDD <darcy.rayner@datadoghq.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

* ci: run django test suite with ddtrace enabled (#2203)

* Run django test suite with ddtrace enabled

* Added pylibmc install to fix build error

* Added django/ddtrace install commands to separate blocks

* Fixed annotation

* Installed generic version of ddtrace instead of checking out

* Added documentation

* Changed working directory, removed inherently failing test

* Reverted working directory change

* Reverted skipping specific test, added pythonpath

* Added more docs

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

* ci: run flask test suite with ddtrace enabled (#2190)

* ci: run flask test suite with ddtrace enabled

* Update .github/workflows/flask.yml

Co-authored-by: Brett Langdon <me@brett.is>

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Co-authored-by: Brett Langdon <brett.langdon@datadoghq.com>
Co-authored-by: Brett Langdon <me@brett.is>
Co-authored-by: Kyle Verhoog <kyle@verhoog.ca>

* docs: add note about pip version requirement (#2220)

* handle logwriter configure (#2219)

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

* test: move __init__ to utils (#2217)

Having code in tests/__init__ makes ddtrace being imported before we have a
chance to do anything, such as calling gevent.monkey.patch_all.

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

* refactor: distributed_tracing_enabled usages to trace_utils helper (#2216)

* Modified trace util helper to account for inconsistencies in distributed_tracing config key, refactored usages

* Refactored traceutils helper

* Made int_config optional in the trace_util helper, updated affected integration files

* Removed erroneous code addition to pyramid/trace.py

* Make tracer tests less noisy (#2221)

monkeypatch.setenv warns if the value given is not an instance of the
base string type.

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

* refactor(worker): replace regular thread by PeriodicThread (#2205)

Co-authored-by: Tahir H. Butt <tahir.butt@datadoghq.com>

* Disable known failing Django tests (#2224)

* Disable known failing Django tests

* fix yaml multiline string

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

* test(botocore): add test case for firehose args (#2227)

This change adds a test case for #2204 to ensure that the params.Records
argument is excluded from the span tags.

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

* refactor: move ddtrace.profiling._attr to ddtrace.utils (#2230)

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

* Adding urllib3 integration (#1148)

* draft of urllib3 integration

* black formatting

* set request tags on the span before the call is made in case of errors

* rename span name and support retry tags based on CR feedback

* add tests and fix a few small bugs

* fix flake8

* add tests and circleci config

* flake8 fixes

* Update ddtrace/contrib/urllib3/__init__.py

Co-Authored-By: Tahir H. Butt <tahir.butt@datadoghq.com>

* use dd compat for urlparse and unparse

* use SpanType constants for type of span

use SpanTypes.HTTP

* fix tests

* fix flake8 from merge conflict resolution

* adapt to latest changes on master

* adopt latest standards for integrations

Use Pin to add patch information to urllib3 requests. Also use riot for configuring the test environment

* apply checklist points

* wip: make tests pass

* set tags on exception

also fixed the tests to override HTTP configuration where required

* docs(urllib3): add urllib3 integration docs

* fix service name handling

* fix copy-pasta errors and improve documentation

* Update ddtrace/contrib/urllib3/__init__.py

Co-authored-by: Kyle Verhoog <kyle@verhoog.ca>

* Update riotfile.py

Co-authored-by: Kyle Verhoog <kyle@verhoog.ca>

* remove unused imports from test sources

* use the new get_argument_value utility

* Update tests/contrib/urllib3/test_urllib3.py

Co-authored-by: Kyle Verhoog <kyle@verhoog.ca>

* Update ddtrace/contrib/urllib3/patch.py

Co-authored-by: Kyle Verhoog <kyle@verhoog.ca>

* commit Tahir's suggestions

* fix TracerTestCase import

Co-authored-by: Tahir H. Butt <tahir.butt@datadoghq.com>
Co-authored-by: Gabriele N. Tornetta <gabriele.tornetta@datadoghq.com>
Co-authored-by: Gabriele N. Tornetta <P403n1x87@users.noreply.github.com>
Co-authored-by: Kyle Verhoog <kyle@verhoog.ca>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Co-authored-by: Yun Kim <35776586+Yun-Kim@users.noreply.github.com>

* Revert writer.write argument change (#2232)

This is a breaking change which we're going to make at a later date.

* Make processor private on Tracer (#2233)

This isn't something we want to make public yet.

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

* fix(profiling): make sure PeriodicCollector start withing service lock (#2214)

This makes sure the periodic collectors are started while the service lock is
held to avoid race conditions while starting the services.

This has no impact in practice since the Profiler class was made thread safe
with #2179.

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

* refactor(forksafe): simplify code (#2222)

This simplifies the code to use a banal decorator approach rather than a weird
lambda-passed-as-argument-based 😉

It also adds typing and an `unregister` function to allow for… unregistering a function!

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

* chore: correct typing hints for span.meta (#2237)

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

* Lambda extension support (#2092)

* feat(agent): support aws lambda extension

* chore: fix flake8 issue with test

* chore: add release note for datadog lambda extension

* chore: fix import order

* chore: fix flake8 issue

* [serverless] fix broken test

* remove unused import

* Add docs, type hints

Co-authored-by: Kyle Verhoog <kyle.verhoog@datadoghq.com>
Co-authored-by: Kyle Verhoog <kyle@verhoog.ca>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Co-authored-by: Yun Kim <35776586+Yun-Kim@users.noreply.github.com>

* fix(span): ensure ID args have the correct type (#2243)

Among all the arguments of the Span constructor, the ones bearing IDs
are the most important type-wise. This change adds checks to ensure that
span_id, trace_id and parent_id all have the expected type.

The rationale for adding the check at construction time is so that this
error could be caught as soon as possible, rather than defer it to later
processing stages, thus avoiding invalid entries in the processing
queue.

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

* fix: only warn if using uwsgi with ddtrace-run (#2202)

* fix: only warn if using uwsgi with ddtrace-run

* remove DeprecationWarning prefix

* update message

Co-authored-by: Brett Langdon <brett.langdon@datadoghq.com>

* Update ISSUE_TEMPLATE.md (#2248)

* Use multiproject pipeline to deploy to reliability env (#2249)

* Use multiproject pipeline for reliability env deploy

* Use default branch

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

* style(pymongo): format pymongo (#2250)

* fix: handle text types in _set_str_tag (#2218)

* chore: correct typing hints for span.meta

* fix: handle text types in _set_str_tag

* chore: correct typing hints for span.meta

* fix: handle text types in _set_str_tag

* fix: handle text types in _set_str_tag

* flake8

* add doc string, fix test

* fix grpc traceback tag

Co-authored-by: Brett Langdon <brett.langdon@datadoghq.com>
Co-authored-by: Gabriele N. Tornetta <P403n1x87@users.noreply.github.com>

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Co-authored-by: Tahir H. Butt <tahir.butt@datadoghq.com>
Co-authored-by: DarcyRaynerDD <darcy.rayner@datadoghq.com>
Co-authored-by: Yun Kim <35776586+Yun-Kim@users.noreply.github.com>
Co-authored-by: Julien Danjou <julien.danjou@datadoghq.com>
Co-authored-by: Brett Langdon <brett.langdon@datadoghq.com>
Co-authored-by: Brett Langdon <me@brett.is>
Co-authored-by: Gabriele N. Tornetta <P403n1x87@users.noreply.github.com>
Co-authored-by: Justin <justin.james.iso@gmail.com>
Co-authored-by: Gabriele N. Tornetta <gabriele.tornetta@datadoghq.com>
Co-authored-by: Darcy Rayner <50632605+DarcyRaynerDD@users.noreply.github.com>
Co-authored-by: Laplie Anderson <randomanderson@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants