Add support for XEvent logging and bump API version to v3 by miljann995 · Pull Request #90 · microsoft/sql-server-language-extensions

Miljan Veljović (miljann995) · 2026-05-27T17:24:48Z

This pull request introduces a new optional host callback API (version 3) for SQL language extensions, enabling extensions to log XEvents back to the host infrastructure. The update defines new callback types and structures in the native interface, implements the callback registration and invocation in both native and managed (.NET) layers, and provides the necessary glue to allow the managed extension to log messages through the host's event system.

The most important changes are:

API and Interface Additions:

Bumped the EXTERNAL_LANGUAGE_EXTENSION_API version from 2 to 3 in sqlexternallanguage.h to indicate the new optional host callback API.
Added the PFunc_ExtensionLogXEvent callback type, the SQLEXTENSION_HOST_CALLBACKS structure, and the SetHostCallbacks API to sqlexternallanguage.h, allowing the host to provide callback functions to extensions.

Native Extension Integration:

Added the SetHostCallbacks function and the g_hostCallbacks pointer to nativecsharpextension.h and implemented their logic in nativecsharpextension.cpp, storing the host callbacks and forwarding them to the managed layer. [1] [2]

Managed Extension Support:

Defined managed representations for the callback delegate and structure in CSharpExtension.cs, and implemented the SetHostCallbacks method to receive and store the callback, enabling managed code to log XEvents through the host.
Updated the Logging utility in Logging.cs to store the host-provided callback and added the LogXEvent method, allowing managed code to log messages through the host's XEvent infrastructure. [1] [2]

Here's the compatibility matrix for general Extension API in terms of host callbacks.

	Host v2	Host v3
Extension v2	`SetHostCallbacks` is not defined in Extension and host does not expect it and does not call it.	`SetHostCallbacks` is not defined in Extension, but host does check it and skips calling it since it's not defined on Extension side.
Extension v3	`SetHostCallbacks` is defined in Extension but host does not expect it and does not call it.	`SetHostCallbacks` is defined on both sides and host calls it.

Here's the compatibility matrix for Host Callbacks API. Here, v2 can be interpreted as vNext. v1 is the current version present in this pull request.

	Host v1	Host v2
Extension v1	Host callbacks struct passed from host contains only `LogXEvent` callback and Extension expects only that one, and saves its address.	Host callbacks struct passed from host contains additional functions apart from `LogXEvent`, but Extension reads only `LogXEvent`, ignoring other callbacks.
Extension v2	Host callbacks struct passed from host contains only `LogXEvent` callback, and Extension expects additional functions apart from `LogXEvent`, but since they are `nullptr` in a struct, they will be skipped.	Host callbacks struct passed from host contains additional functions apart from `LogXEvent`, and Extension expects additional functions apart from `LogXEvent`, saves their addresses and calls them.

Copilot

Pull request overview

This PR introduces a v3 extension interface update to support optional host callbacks, enabling extensions (specifically the .NET C# extension) to log XEvents back through the host infrastructure.

Changes:

Bumped EXTERNAL_LANGUAGE_EXTENSION_API from 2 to 3 and added a new optional SetHostCallbacks API + callback struct/type in the shared native interface header.
Implemented native SetHostCallbacks in the .NET C# extension and forwarded the callback struct to the managed layer.
Added managed callback delegates/structs plus a Logging.LogXEvent(...) helper that invokes the host-provided callback.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
extension-host/include/sqlexternallanguage.h	Bumps API version to v3; adds host callback typedef/struct and `SetHostCallbacks` API declaration.
language-extensions/dotnet-core-CSharp/include/nativecsharpextension.h	Declares `SetHostCallbacks` export and stores a host-callbacks pointer.
language-extensions/dotnet-core-CSharp/src/native/nativecsharpextension.cpp	Implements native `SetHostCallbacks`; resets host callbacks on cleanup.
language-extensions/dotnet-core-CSharp/src/managed/CSharpExtension.cs	Adds managed callback delegate/struct and implements managed `SetHostCallbacks`.
language-extensions/dotnet-core-CSharp/src/managed/utils/Logging.cs	Stores callback delegate and adds `LogXEvent` helper for managed code.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

SicongLiu2000

Review Summary — XEvent Logging Callback API (v3)

Nice feature — clean API surface and good test coverage. Three items I'd want addressed before merge, plus several medium-severity hardening suggestions:

Must-fix (High)

TOCTOU on _logXEventCallback (Logging.cs) — snapshot to local before null-check and invocation to prevent NRE from concurrent SetLogXEventCallback(null).
Use-after-free in Cleanup() (nativecsharpextension.cpp) — must clear the managed-side callback delegate before tearing down the .NET runtime, otherwise background threads can invoke a stale host function pointer.
char* vs byte* delegate type mismatch (CSharpExtension.cs) — delegate declares char* (2-byte UTF-16) but actually receives byte* (1-byte UTF-8). Should be byte* to match the native SQLCHAR*.

Should-fix (Medium)

Dangling pointer — g_hostCallbacks stores borrowed pointer; copy the struct or remove the dead global.
fixed on empty array — yields null pointer passed to native code; guard the empty-message case.
static globals in header — pre-existing issue, but the new g_hostCallbacks follows the pattern; extern + single definition is correct.

Consider (Low)

Version check could use a MAX_SUPPORTED constant for future extensibility.
ulong vs nuint for SQLULEN if 32-bit is ever a target.

See inline comments for details and suggested fixes.

SicongLiu2000

All 8 findings addressed — TOCTOU snapshot, use-after-free ordering, type mismatch, dangling pointer copy, fixed-null guard, extern globals, version constant, and typo. Good test coverage with the new GTest suite. LGTM 👍

Copilot

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 6 comments.

This reverts commit 7b6bdb3.

…nsion name otherwise

MHN (monamaki)

Approving to deliver.
please ensure addressing comments and also we need to follow this model:
RE: Extension API v3 versioning — negotiate-down vs strict check — need your input

…ax=2); keep decimal (#83), make v3 host-callbacks (#90) dormant

…ls + Cleanup teardown). Native .so was exporting/executing v3 code that aborts on the v2 host; managed decimal + v2 pin retained

…e + host-callback headers; API stays v2). Native .so now byte-identical to proven-working baseline; managed decimal retained

Reconcile the Linux support with main, which has since added DECIMAL (microsoft#83), ZIP archive InstallExternalLibrary (microsoft#85), and SetHostCallbacks/API v3 (microsoft#90). Conflict + fallout resolutions (Linux portability for the new main code): - CMakeLists.txt: keep the WIN32/else split (Linux gtest-from-source, -fshort-wchar, dl/pthread linking); drop main''s unconditional -D_WIN64/_WINDOWS which broke Linux. - CSharpExtensionApiTests.cpp GetHandles(): resolve the InstallExternalLibrary/ UninstallExternalLibrary symbol lookups for both the Windows (GetProcAddress) and Linux (dlsym) branches. - CSharpLibraryTests.cpp (new, microsoft#85): add cross-platform GetExecutablePath() helper (readlink on Linux), guard LocalFree/HLOCAL with free() on Linux. - CSharpSetHostCallbacksTests.cpp (new, microsoft#90): make RESOLVE_SET_HOST_CALLBACKS use dlsym on Linux. - run-...-test.sh: export ENL_ROOT so the test binary can locate test_packages.

Miljan Veljović (miljann995) changed the title ~~feat: add v3 of extension api with host callbacks~~ Add support for XEvent logging and bump API version to v3 May 27, 2026

Miljan Veljović (miljann995) marked this pull request as ready for review May 27, 2026 18:28

Copilot AI review requested due to automatic review settings May 27, 2026 18:28

Copilot started reviewing on behalf of Miljan Veljović (miljann995) May 27, 2026 18:28 View session

Copilot AI reviewed May 27, 2026

View reviewed changes

MHN (monamaki) requested a review from Copilot May 28, 2026 00:01

Copilot started reviewing on behalf of MHN (monamaki) May 28, 2026 00:03 View session

Copilot AI reviewed May 28, 2026

View reviewed changes

Comment thread language-extensions/dotnet-core-CSharp/src/managed/CSharpExtension.cs

Comment thread language-extensions/dotnet-core-CSharp/src/managed/CSharpExtension.cs

This comment was marked as off-topic.

Sign in to view

SicongLiu2000 requested changes May 28, 2026

View reviewed changes

Miljan Veljović (miljann995) requested a review from SicongLiu2000 May 28, 2026 21:57

SicongLiu2000 approved these changes May 28, 2026

View reviewed changes

MHN (monamaki) requested a review from Copilot May 29, 2026 01:05

Copilot started reviewing on behalf of MHN (monamaki) May 29, 2026 01:05 View session

MHN (monamaki) requested changes May 29, 2026

View reviewed changes

Copilot AI reviewed May 29, 2026

View reviewed changes

Miljan Veljović (miljann995) force-pushed the dev/mveljovic/host-callback-api branch from c3c4392 to 875619b Compare May 29, 2026 13:02

Miljan Veljović (miljann995) added 12 commits May 29, 2026 15:06

feat: add v3 of extension api with host callbacks

15aa08e

feat: remove extension name from parameters list

ffc94f4

Revert "feat: remove extension name from parameters list"

effc829

This reverts commit 7b6bdb3.

fix: adjust parameters

4aa44e8

chore: comments

d62a9de

feat: add test

0adb869

chore: comments

058e1a0

fix: add version check

6c2ba56

fix: set LogXEvent callback to nullptr if not provided by the host

7060815

fix: address nullptr concern

7329d03

fix: add version check

8d50d54

fix: address code review comments

80a7076

Miljan Veljović (miljann995) added 10 commits May 29, 2026 15:06

fix: address code review comments

44b3f23

fix: avoid race condition on concurrent cleanups

87b8636

fix: type mismatch

dfc2d4d

fix: address race condition concerns

01d9889

fix: avoid dangling pointer of host callbacks

456aa78

feat: extract trace level into separate header file

011520d

fix: extract trace level enum in a separate header file

e726883

fix: use caller-supplied extension name, if provided, or default exte…

74d7a5b

…nsion name otherwise

fix: forward compatibility on host callback versions

b5c7417

fix: forward compatibility test

2f9a8c9

Miljan Veljović (miljann995) force-pushed the dev/mveljovic/host-callback-api branch from 875619b to 2f9a8c9 Compare May 29, 2026 13:06

Miljan Veljović (miljann995) requested a review from MHN (monamaki) May 29, 2026 15:43

MHN (monamaki) requested changes Jun 1, 2026

View reviewed changes

Comment thread language-extensions/dotnet-core-CSharp/src/managed/CSharpExtension.cs

MHN (monamaki) reviewed Jun 1, 2026

View reviewed changes

Comment thread language-extensions/dotnet-core-CSharp/src/managed/CSharpExtension.cs

MHN (monamaki) approved these changes Jun 1, 2026

View reviewed changes

fix: adjust managed host callbacks struct to match the native one

6c5fbda

Miljan Veljović (miljann995) added this pull request to the merge queue Jun 1, 2026

Merged via the queue into main with commit 7d1dadb Jun 1, 2026
5 checks passed

Miljan Veljović (miljann995) deleted the dev/mveljovic/host-callback-api branch June 1, 2026 14:09

SicongLiu2000 added a commit that referenced this pull request Jun 26, 2026

Pin extension API to v2 to match mssql-server adhoc Linux host (min/m…

c1adb3d

…ax=2); keep decimal (#83), make v3 host-callbacks (#90) dormant

SicongLiu2000 added a commit that referenced this pull request Jun 26, 2026

Remove PR #90 v3 host-callbacks native code (SetHostCallbacks + globa…

d4d8e3e

…ls + Cleanup teardown). Native .so was exporting/executing v3 code that aborts on the v2 host; managed decimal + v2 pin retained

SicongLiu2000 added a commit that referenced this pull request Jun 26, 2026

Reset native/host source to baseline a13ff97 (drop all #90 native cod…

58a9bbf

…e + host-callback headers; API stays v2). Native .so now byte-identical to proven-working baseline; managed decimal retained

Uh oh!

Conversation

Miljan Veljović (miljann995) commented May 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

SicongLiu2000 left a comment

Choose a reason for hiding this comment

Review Summary — XEvent Logging Callback API (v3)

Must-fix (High)

Should-fix (Medium)

Consider (Low)

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

SicongLiu2000 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

MHN (monamaki) left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Miljan Veljović (miljann995) commented May 27, 2026 •

edited

Loading