Implement Python API for submitting logs #26978

ofek · 2024-06-22T19:33:21Z

Motivation

Expose the new functionality to Python checks #26753

Additional Notes

After the base PR is merged this PR will re-target main

rtrieu · 2024-06-24T14:04:11Z

releasenotes/notes/submit-logs-from-python-checks-4fa62927ae2b86a2.yaml

+---
+enhancements:
+ - |
+ Implement API that allows Python checks to send logs for


Suggested change

Implement API that allows Python checks to send logs for

Implements API that allows Python checks to send logs for

I'm not sure this is desirable as these entries get directly put on the Agent release change log. In that context, it's less so that we are describing this specific patch but rather as a whole. Does that make sense?

rtrieu

Minor feedback but otherwise looks good!

pr-commenter · 2024-07-02T03:31:17Z

Test changes on VM

Use this command from test-infra-definitions to manually test this PR changes on a VM:

inv create-vm --pipeline-id=40520993 --os-family=ubuntu

Note: This applies to commit a775579

pr-commenter · 2024-07-02T04:10:48Z

Regression Detector

Regression Detector Results

Run ID: 17736aeb-9360-4daa-92b7-fbc816faa9fb Metrics dashboard Target profiles

Baseline: fc52f43
Comparison: a775579

Performance changes are noted in the perf column of each table:

✅ = significantly better comparison variant performance
❌ = significantly worse comparison variant performance
➖ = no significant change in performance

No significant changes in experiment optimization goals

Confidence level: 90.00%
Effect size tolerance: |Δ mean %| ≥ 5.00%

There were no significant changes in experiment optimization goals at this confidence level and effect size tolerance.

Fine details of change detection per experiment

perf	experiment	goal	Δ mean %	Δ mean % CI	links
➖	otel_to_otel_logs	ingress throughput	+0.10	[-0.71, +0.91]	Logs
➖	tcp_dd_logs_filter_exclude	ingress throughput	-0.00	[-0.01, +0.01]	Logs
➖	uds_dogstatsd_to_api	ingress throughput	-0.00	[-0.00, +0.00]	Logs
➖	file_tree	memory utilization	-0.41	[-0.49, -0.33]	Logs
➖	basic_py_check	% cpu utilization	-0.63	[-3.14, +1.88]	Logs
➖	idle	memory utilization	-0.64	[-0.68, -0.60]	Logs
➖	uds_dogstatsd_to_api_cpu	% cpu utilization	-2.21	[-3.09, -1.32]	Logs
➖	pycheck_1000_100byte_tags	% cpu utilization	-2.22	[-6.90, +2.45]	Logs
➖	tcp_syslog_to_blackhole	ingress throughput	-3.93	[-16.32, +8.46]	Logs

Explanation

A regression test is an A/B test of target performance in a repeatable rig, where "performance" is measured as "comparison variant minus baseline variant" for an optimization goal (e.g., ingress throughput). Due to intrinsic variability in measuring that goal, we can only estimate its mean value for each experiment; we report uncertainty in that value as a 90.00% confidence interval denoted "Δ mean % CI".

For each experiment, we decide whether a change in performance is a "regression" -- a change worth investigating further -- if all of the following criteria are true:

Its estimated |Δ mean %| ≥ 5.00%, indicating the change is big enough to merit a closer look.
Its 90.00% confidence interval "Δ mean % CI" does not contain zero, indicating that if our statistical model is accurate, there is at least a 90.00% chance there is a difference in performance between baseline and comparison variants.
Its configuration does not mark it "erratic".

chouquette · 2024-07-23T08:20:00Z

rtloader/common/builtins/datadog_agent.c

+ Py_RETURN_NONE;
+ }
+
+ char *log_line, *check_id;


If I understand Python's documentation correctly, these should be const char* as the memory is owned by python

This would imply the cb_send_log_t type should also be updated to pass both parameters as pointers to constant chars

This is the same as every function in this file; are you certain?

Quite certain:

Other formats take a str or a read-only bytes-like object, such as bytes, and provide a const char * pointer to its buffer. In this case the buffer is “borrowed”: it is managed by the corresponding Python object, and shares the lifetime of this object. You won’t have to release any memory yourself.

https://docs.python.org/3/c-api/arg.html#strings-and-buffers

It doesn't make an actual difference for most purposes, but having a const char' makes it clear that we don't own that memory

Thanks! This will be done in a follow-up PR.

rtloader/common/builtins/datadog_agent.c

chouquette · 2024-07-23T08:25:38Z

rtloader/common/builtins/datadog_agent.c

+
+ This function is callable as the `datadog_agent.send_log` Python method and
+ uses the `cb_send_log()` callback to retrieve the value from the agent
+ with CGO. If the callback has not been set `None` will be returned.


If the callback has not been set None will be returned.

This seems to imply that a different result would be returned if a callback was set, which isn't the case

Every function has this documentation; are they all incorrect?

I don't think it's incorrect, but the last sentence seems misleading since None is always returned.
Now to be clear, this is definitely in the nit picking realm and can be ignored

comp/logs/integrations/impl/integrations.go

comp/logs/integrations/def/component.go

comp/logs/integrations/impl/integrations_test.go

cmd/agent/subcommands/run/command.go

comp/logs/integrations/fx/fx.go

comp/logs/integrations/mock/mock.go

pgimalac · 2024-07-23T09:23:47Z

I realized that most of my comments are about code from #26753 which is already merged, you might want to pull main to remove those commits

ofek · 2024-07-31T12:53:03Z

/merge

dd-devflow · 2024-07-31T12:53:16Z

🚂 MergeQueue: pull request added to the queue

The median merge time in main is 22m.

Use /merge -c to cancel this operation!

dd-devflow · 2024-07-31T12:56:40Z

🚨 MergeQueue: This merge request is in error

Gitlab pipeline didn't start on its own and we were unable to create it... Please retry.

If you need support, contact us on Slack #devflow with those details!

ofek · 2024-07-31T13:55:07Z

/merge

dd-devflow · 2024-07-31T13:55:16Z

🚂 MergeQueue: pull request added to the queue

The median merge time in main is 22m.

Use /merge -c to cancel this operation!

This reverts commit eee54f7.

ofek added team/integrations team/agent-metrics-logs qa/done Skip QA week as QA was done before merge and regressions are covered by tests labels Jun 22, 2024

ofek requested review from a team as code owners June 22, 2024 19:33

rtrieu reviewed Jun 24, 2024

View reviewed changes

rtrieu approved these changes Jun 24, 2024

View reviewed changes

soberpeach mentioned this pull request Jun 28, 2024

Added logs_from_integrations component #26753

Merged

ofek force-pushed the ofek/logs branch from f2ef27a to 973b579 Compare July 2, 2024 03:04

gh123man approved these changes Jul 2, 2024

View reviewed changes

ofek force-pushed the ofek/logs branch 2 times, most recently from 4cd843a to b489f51 Compare July 5, 2024 14:55

ofek mentioned this pull request Jul 10, 2024

Support log submission from checks DataDog/integrations-core#18019

Merged

ofek force-pushed the ofek/logs branch from b489f51 to 9008240 Compare July 10, 2024 17:18

Base automatically changed from lucas.liseth/go-interface-for-logs-from-integrations to main July 22, 2024 14:31

dd-mergequeue bot requested review from a team as code owners July 22, 2024 14:31

chouquette reviewed Jul 23, 2024

View reviewed changes

pgimalac reviewed Jul 23, 2024

View reviewed changes

comp/logs/integrations/fx/fx.go Outdated Show resolved Hide resolved

comp/logs/integrations/mock/mock.go Outdated Show resolved Hide resolved

Implement Python API for submitting logs

a775579

ofek force-pushed the ofek/logs branch from 9008240 to a775579 Compare July 30, 2024 22:09

dd-mergequeue bot merged commit eee54f7 into main Jul 31, 2024
220 checks passed

dd-mergequeue bot deleted the ofek/logs branch July 31, 2024 14:20

github-actions bot added this to the 7.57.0 milestone Jul 31, 2024

carlosroman added a commit that referenced this pull request Aug 2, 2024

Revert "Implement Python API for submitting logs (#26978)"

8a0a17f

This reverts commit eee54f7.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement Python API for submitting logs #26978

Implement Python API for submitting logs #26978

ofek commented Jun 22, 2024

rtrieu Jun 24, 2024

ofek Jul 5, 2024

rtrieu left a comment

pr-commenter bot commented Jul 2, 2024 •

edited

Loading

pr-commenter bot commented Jul 2, 2024 •

edited

Loading

Fine details of change detection per experiment

Explanation

chouquette Jul 23, 2024

ofek Jul 30, 2024

chouquette Jul 31, 2024

ofek Jul 31, 2024

chouquette Jul 23, 2024

ofek Jul 30, 2024

chouquette Jul 31, 2024

pgimalac commented Jul 23, 2024

ofek commented Jul 31, 2024

dd-devflow bot commented Jul 31, 2024

dd-devflow bot commented Jul 31, 2024

ofek commented Jul 31, 2024

dd-devflow bot commented Jul 31, 2024

	Implement API that allows Python checks to send logs for
	Implements API that allows Python checks to send logs for

Implement Python API for submitting logs #26978

Implement Python API for submitting logs #26978

Conversation

ofek commented Jun 22, 2024

Motivation

Additional Notes

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rtrieu left a comment

Choose a reason for hiding this comment

pr-commenter bot commented Jul 2, 2024 • edited Loading

Test changes on VM

pr-commenter bot commented Jul 2, 2024 • edited Loading

Regression Detector

Regression Detector Results

No significant changes in experiment optimization goals

Fine details of change detection per experiment

Explanation

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pgimalac commented Jul 23, 2024

ofek commented Jul 31, 2024

dd-devflow bot commented Jul 31, 2024

dd-devflow bot commented Jul 31, 2024

ofek commented Jul 31, 2024

dd-devflow bot commented Jul 31, 2024

pr-commenter bot commented Jul 2, 2024 •

edited

Loading

pr-commenter bot commented Jul 2, 2024 •

edited

Loading