Skip to content

Properly initialize content handler logging#828

Closed
AndreRicardo-Zoetis wants to merge 6 commits intoAzure:developfrom
AndreRicardo-Zoetis:fix-780-missing-curl-logs
Closed

Properly initialize content handler logging#828
AndreRicardo-Zoetis wants to merge 6 commits intoAzure:developfrom
AndreRicardo-Zoetis:fix-780-missing-curl-logs

Conversation

@AndreRicardo-Zoetis
Copy link
Copy Markdown
Contributor

@AndreRicardo-Zoetis AndreRicardo-Zoetis commented Feb 26, 2026

I know you just cleared all Pull Requests, congrats! 😅

Fix #780 show curl logs.

While testing noticed that logLevel was not passed to contentDownloader, needed to pass logLevel so it works as expected for different -l parameters.

image

do-client logs were also missing, added initialization now.

deliveryoptimization is harder to test for us but did a full build and could see the do_download logs now:

image

Other changes:

  • Added clean up logic when the library is being unloaded and renamed variable for consistency.

@AndreRicardo-Zoetis AndreRicardo-Zoetis changed the title Properly initialize curl content handler logging Properly initialize content handler logging Feb 26, 2026
@Nox-MSFT
Copy link
Copy Markdown
Collaborator

Nox-MSFT commented Mar 6, 2026

@AndreRicardo-Zoetis - thanks for the PR. The changes look good, but I’m a bit concerned this could break others who have built custom content download handlers. We may want to consider a backward‑compatible approach (e.g., build flag or runtime check) to support both old and new ABIs.

@AndreRicardo-Zoetis
Copy link
Copy Markdown
Contributor Author

AndreRicardo-Zoetis commented Mar 27, 2026

@AndreRicardo-Zoetis - thanks for the PR. The changes look good, but I’m a bit concerned this could break others who have built custom content download handlers. We may want to consider a backward‑compatible approach (e.g., build flag or runtime check) to support both old and new ABIs.

Ok, but won't they have the same problem of the severity not being passed? Are these customers seeing their logs without the parameter?

Also the initializeData is only needed from what I can tell in do-client, while the log level is used by both.

@Nox-MSFT
Copy link
Copy Markdown
Collaborator

@AndreRicardo-Zoetis , thanks for the PR. I've a proposal for additional changes on top of your PR. Please take a look #861 and let us know if you have any feedback/suggestion. Thank you.

Nox-MSFT added a commit that referenced this pull request Apr 22, 2026
…861)

* Properly initialize curl content handler logging

* Pass logLevel to contentDownloader.

* Fix do-client logging initialization.

* Remove comment.

* Update header file.

* Call Cleanup when the lib is being Unloaded.

* feat: Add backward-compatible V2 content downloader contract

Enhance PR #828's logging initialization for content downloaders with
proper backward compatibility via a V2 contract model:

- Add V2 contract version (2.0) constants and ADUC_ContractUtils_IsV2Contract()
- Add InitializeV1Proc typedef for V1 ABI (1 arg) alongside V2's InitializeProc (2 args)
- Branch InitializeContentDownloader on contract version: V1 calls with 1 arg, V2 with 2 args
- Remove Cleanup from required symbols array so V1 extensions still load
- Only call Cleanup on the content downloader lib (not all libs) and only for V2
- Update Download() to accept both V1 and V2 contract versions
- Update curl and DO downloaders to report V2 contract
- Add unit tests for V1/V2/unsupported contract version routing

Addresses ABI compatibility concerns raised in #828.


* docs: Document V2 content downloader contract

Update extension-contract-versions.md with V1 vs V2 content downloader
contract details (Initialize signature change, Cleanup export).

Update device-update-agent-extensibility-points.md custom content
downloader section with V1/V2 guidance.

* test: Fix unit tests after develop merge and V2 contract adoption

- extension_manager_ut.cpp: pass ADUC_LOG_DEBUG to InitializeContentDownloader
  callsites (build break after logLevel param was added)
- extension_manager_ut.cpp, extension_manager_download_test_case.cpp: use
  {3,0} instead of {2,0} for the 'unsupported contract version' scenarios,
  since {2,0} is now a supported V2 contract
- curl_content_downloader_ut.cpp: update GetContractInfo test to expect V2,
  matching curl downloader's reported contract version

All 744 unit tests pass.

---------

Co-authored-by: Andre Ricardo <andre.ricardo@zoetis.com>
@Nox-MSFT
Copy link
Copy Markdown
Collaborator

@AndreRicardo-Zoetis, I’m closing this PR since the changes have already been incorporated in this commit: #861

@Nox-MSFT Nox-MSFT closed this Apr 22, 2026
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.

curl content downloader logs not printing

2 participants