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

Add background upload configuration info to batch deleted telemetry #2039

Merged
merged 7 commits into from
Oct 17, 2024

Conversation

maciejburda
Copy link
Contributor

@maciejburda maciejburda commented Sep 5, 2024

What and why?

Enriches Batch Deleted telemetry metric with backgroundTaskEnabled configuration info.
This is needed for measuring batch delivery success rate based on existence of the configuration.

How?

I have added new field background_tasks_enabled.

in_background is still hardcoded to false, because knowing if the deletion happen in the background is not trivial.
Although I think we're mostly interested in measuring the difference is success rate for two different configurations, rather than knowing if batch file was deleted in background/foreground.

Review checklist

  • Feature or bugfix MUST have appropriate tests (unit, integration)
  • Make sure each commit and the PR mention the Issue number or JIRA reference
  • Add CHANGELOG entry for user facing changes

@maciejburda maciejburda marked this pull request as ready for review September 5, 2024 13:22
@maciejburda maciejburda requested review from a team as code owners September 5, 2024 13:22
@datadog-datadog-prod-us1
Copy link

datadog-datadog-prod-us1 bot commented Sep 5, 2024

Datadog Report

Branch report: maciey/add-backgroundtask-telemetry
Commit report: 2ddb942
Test service: dd-sdk-ios

✅ 0 Failed, 3491 Passed, 0 Skipped, 2m 19.07s Total Time
🔻 Test Sessions change in coverage: 3 decreased, 3 increased, 8 no change

🔻 Code Coverage Decreases vs Default Branch (3)

  • test DatadogInternalTests tvOS 79.72% (-0.03%) - Details
  • test DatadogTraceTests tvOS 54.24% (-0.02%) - Details
  • test DatadogCrashReportingTests tvOS 26.77% (-0.02%) - Details

Copy link
Collaborator

@ncreated ncreated left a comment

Choose a reason for hiding this comment

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

I like the idea of collecting this attribute 👍. Although we should introduce a new field rather than re-using existing one for different purpose. Otherwise we will invalidate the spec through this PR.

@maciejburda maciejburda force-pushed the maciey/add-backgroundtask-telemetry branch from 2510312 to 17ef89b Compare October 3, 2024 09:22
@maciejburda
Copy link
Contributor Author

/merge

@dd-devflow
Copy link

dd-devflow bot commented Oct 17, 2024

🚂 MergeQueue: pull request added to the queue

The median merge time in develop is 0s.

Use /merge -c to cancel this operation!

@dd-devflow
Copy link

dd-devflow bot commented Oct 17, 2024

MergeQueue: This merge request was updated

This PR is rejected because it was updated

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

@maciejburda
Copy link
Contributor Author

/merge

@dd-devflow
Copy link

dd-devflow bot commented Oct 17, 2024

🚂 MergeQueue: waiting for PR to be ready

This merge request is not mergeable yet, because of pending checks/missing approvals. It will be added to the queue as soon as checks pass and/or get approvals.
Note: if you pushed new commits since the last approval, you may need additional approval.
You can remove it from the waiting list with /remove command.

Use /merge -c to cancel this operation!

@dd-devflow
Copy link

dd-devflow bot commented Oct 17, 2024

🚂 MergeQueue: pull request added to the queue

The median merge time in develop is 0s.

Use /merge -c to cancel this operation!

@dd-mergequeue dd-mergequeue bot merged commit a52c5fa into develop Oct 17, 2024
16 checks passed
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.

3 participants