Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
107 changes: 107 additions & 0 deletions content/docs/analyzers/ApplicationCop/AC0033.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
+++
title = 'Rest Client requires an Http Client Handler implementation'
linkTitle = 'AC0033'

[params]
id = 'AC0033'
severity = 'Warning'
category = 'Design'
codeAction = false
ignoreObsolete = true
+++

`Codeunit "Rest Client"` does not send requests itself. It passes every request to an implementation of the `"Http Client Handler"` interface. When no handler is given, it uses the System Application's own `"Http Client Handler"` codeunit, which calls `HttpClient.Send`. The outgoing call then belongs to the System Application, not to the app that built the request.

In a sandbox, allowing HTTP client requests for the System Application therefore allows them for every extension that uses the Rest Client. The outgoing web service request telemetry for those calls goes to Microsoft's telemetry instead of the app publisher's. Tests cannot substitute a response either, because the handler is the point where a test replaces the real call. Implement `"Http Client Handler"` in the app and pass it to the Rest Client.

### Example

{{< highlight al "hl_lines=1" >}}
codeunit 50100 "Exchange Rate Sync" // Rest Client requires an Http Client Handler implementation [AC0033]
{
procedure GetRates(CurrencyCode: Code[10]): JsonToken
var
RestClient: Codeunit "Rest Client";
begin
exit(RestClient.GetAsJson('https://api.example.com/rates/' + CurrencyCode));
end;
}
{{< /highlight >}}

Add a codeunit that implements `"Http Client Handler"` and pass it to `RestClient.Initialize`:

{{< highlight al "hl_lines=1-12 19 21" >}}
codeunit 50110 "Exchange Rate Http Handler" implements "Http Client Handler"
{
Access = Internal;

procedure Send(CurrHttpClientInstance: HttpClient; HttpRequestMessage: Codeunit "Http Request Message"; var HttpResponseMessage: Codeunit "Http Response Message") Success: Boolean;
var
ResponseMessage: HttpResponseMessage;
begin
Success := CurrHttpClientInstance.Send(HttpRequestMessage.GetHttpRequestMessage(), ResponseMessage);
HttpResponseMessage := HttpResponseMessage.Create(ResponseMessage);
end;
}

codeunit 50100 "Exchange Rate Sync"
{
procedure GetRates(CurrencyCode: Code[10]): JsonToken
var
RestClient: Codeunit "Rest Client";
HttpHandler: Codeunit "Exchange Rate Http Handler";
begin
RestClient.Initialize(HttpHandler);
exit(RestClient.GetAsJson('https://api.example.com/rates/' + CurrencyCode));
end;
}
{{< /highlight >}}

`RestClient.Create(HttpHandler)` is equivalent. It returns the initialized Rest Client, which suits code that assigns the client in one statement:

{{< highlight al >}}
RestClient := RestClient.Create(HttpHandler);
{{< /highlight >}}

`Initialize()`, `Initialize(HttpAuthentication)`, `Create()` and `Create(HttpAuthentication)` all use the default handler, and so does a Rest Client that is used without being initialized. AC0033 only checks that the app contains a handler implementation. It does not check that each Rest Client receives it.

### When the diagnostic is reported

- An object declares a global variable, local variable, parameter or return value of type `Codeunit "Rest Client"`. Locals in triggers of fields, actions, controls, report data items and XMLport elements count.
- No codeunit in the same app implements `"Http Client Handler"`.
- The Rest Client codeunit is matched on both its ID (2350) and its name.
- One diagnostic is reported per object, on the object name, regardless of how many Rest Client variables the object declares.

### Exception

Test codeunits (`Subtype = Test` or `TestRunner`) and obsolete objects are not reported.

Only codeunits in the app being compiled count as an implementation. When the handler lives in a dependency app from the same publisher, such as a shared foundation app, the calls are still attributed to that publisher, but AC0033 does not see the handler. Suppress the diagnostic on the object:

{{< highlight al >}}
// The "Http Client Handler" implementation is provided by the foundation app,
// a dependency from the same publisher; AC0033 only sees codeunits in this app.
#pragma warning disable AC0033
codeunit 50100 "Exchange Rate Sync"
#pragma warning restore AC0033
{
procedure GetRates(CurrencyCode: Code[10]): JsonToken
var
RestClient: Codeunit "Rest Client";
FoundationHttpHandler: Codeunit "Foundation Http Handler";
begin
RestClient.Initialize(FoundationHttpHandler);
exit(RestClient.GetAsJson('https://api.example.com/rates/' + CurrencyCode));
end;
}
{{< /highlight >}}

For an app that relies on a shared handler throughout, disable the rule in the ruleset instead.

[AC0034](../ac0034/) applies the same check to `Codeunit Telemetry` and `Codeunit "Feature Telemetry"`.

### See also

- [Codeunit "Rest Client"](https://learn.microsoft.com/en-us/dynamics365/business-central/application/system-application/codeunit/system.restclient.rest-client) on Microsoft Learn
- [Interface "Http Client Handler"](https://learn.microsoft.com/en-us/dynamics365/business-central/application/system-application/interface/system.restclient.http-client-handler) on Microsoft Learn
- [Prefer the Rest Client module over the native HttpClient](https://github.com/ALCops/Analyzers/discussions/405) on GitHub Discussions
102 changes: 102 additions & 0 deletions content/docs/analyzers/ApplicationCop/AC0034.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
+++
title = 'Telemetry requires a Telemetry Logger implementation'
linkTitle = 'AC0034'

[params]
id = 'AC0034'
severity = 'Warning'
category = 'Design'
codeAction = false
ignoreObsolete = true
+++

`Codeunit Telemetry` and `Codeunit "Feature Telemetry"` do not call `Session.LogMessage` for the calling app. They look up the `"Telemetry Logger"` registered for the calling app's publisher and hand the message to it. A logger is registered by a subscriber to the `OnRegisterTelemetryLogger` event of `Codeunit "Telemetry Loggers"`.

> Every publisher needs to have an implementation of the "Telemetry Logger" interface and a subscriber to "Telemetry Loggers".OnRegisterTelemetryLogger event in one of their apps in order for this codeunit to work as expected

— [Codeunit Telemetry](https://learn.microsoft.com/en-us/dynamics365/business-central/application/system-application/codeunit/system.telemetry.telemetry) on Microsoft Learn

When no logger is registered for the publisher, `"Telemetry Loggers Impl."` logs warning `0000G7K` and discards the message. The publisher's Application Insights resource never receives the event:

> An app from publisher %1 is sending telemetry, but there is no registered telemetry logger for this publisher.

— `NoPublisherErr` label in [`"Telemetry Loggers Impl."`](https://github.com/microsoft/BCApps/blob/main/src/System%20Application/App/Telemetry/src/Logging/TelemetryLoggersImpl.Codeunit.al) in the System Application

Implement `"Telemetry Logger"` in the app and register it in an `OnRegisterTelemetryLogger` subscriber.

### Example

{{< highlight al "hl_lines=1" >}}
codeunit 50101 "Sales Order Telemetry" // Telemetry requires a Telemetry Logger implementation [AC0034]
{
procedure LogOrderReleased()
var
FeatureTelemetry: Codeunit "Feature Telemetry";
begin
FeatureTelemetry.LogUsage('0000ABC', 'Sales Order Release', 'Sales order released');
end;
}
{{< /highlight >}}

`"Sales Order Telemetry"` stays as it is. Add a codeunit that implements `"Telemetry Logger"` and registers itself:

{{< highlight al "hl_lines=1-17" >}}
codeunit 50111 "App Telemetry Logger" implements "Telemetry Logger"
{
Access = Internal;

procedure LogMessage(EventId: Text; Message: Text; Verbosity: Verbosity; DataClassification: DataClassification; TelemetryScope: TelemetryScope; CustomDimensions: Dictionary of [Text, Text])
begin
Session.LogMessage(EventId, Message, Verbosity, DataClassification, TelemetryScope, CustomDimensions);
end;

[EventSubscriber(ObjectType::Codeunit, Codeunit::"Telemetry Loggers", 'OnRegisterTelemetryLogger', '', true, true)]
local procedure OnRegisterTelemetryLogger(var Sender: Codeunit "Telemetry Loggers")
var
TelemetryLogger: Codeunit "App Telemetry Logger";
begin
Sender.Register(TelemetryLogger);
end;
}
{{< /highlight >}}

The registration is per publisher, not per app: the System Application expects exactly one `"Telemetry Logger"` implementation registered per app publisher. AC0034 only checks that the app contains an implementation. It does not check that the implementation is registered.

### When the diagnostic is reported

- An object declares a global variable, local variable, parameter or return value of type `Codeunit Telemetry` or `Codeunit "Feature Telemetry"`. Locals in triggers of fields, actions, controls, report data items and XMLport elements count.
- No codeunit in the same app implements `"Telemetry Logger"`.
- The telemetry codeunits are matched on both their ID (8711 for `Telemetry`, 8703 for `"Feature Telemetry"`) and their name.
- One diagnostic is reported per object, on the object name, regardless of how many telemetry variables the object declares.

### Exception

Test codeunits (`Subtype = Test` or `TestRunner`) and obsolete objects are not reported.

Only codeunits in the app being compiled count as an implementation. Because the logger is registered per publisher, a logger in a dependency app from the same publisher, such as a shared foundation app, serves every app of that publisher, but AC0034 does not see it. Suppress the diagnostic on the object:

{{< highlight al >}}
// The "Telemetry Logger" for this publisher is implemented and registered in the
// foundation app, a dependency from the same publisher: one logger per publisher.
#pragma warning disable AC0034
codeunit 50101 "Sales Order Telemetry"
#pragma warning restore AC0034
{
procedure LogOrderReleased()
var
FeatureTelemetry: Codeunit "Feature Telemetry";
begin
FeatureTelemetry.LogUsage('0000ABC', 'Sales Order Release', 'Sales order released');
end;
}
{{< /highlight >}}

For an app that relies on a shared logger throughout, disable the rule in the ruleset instead.

[AC0033](../ac0033/) applies the same check to `Codeunit "Rest Client"` and `"Http Client Handler"`.

### See also

- [Interface "Telemetry Logger"](https://learn.microsoft.com/en-us/dynamics365/business-central/application/system-application/interface/system.telemetry.telemetry-logger) on Microsoft Learn
- [Codeunit "Feature Telemetry"](https://learn.microsoft.com/en-us/dynamics365/business-central/application/system-application/codeunit/system.telemetry.feature-telemetry) on Microsoft Learn
- [Prefer the Rest Client module over the native HttpClient](https://github.com/ALCops/Analyzers/discussions/405) on GitHub Discussions
2 changes: 2 additions & 0 deletions content/docs/analyzers/ApplicationCop/_index.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,5 +42,7 @@ ApplicationCop inspects how objects are modeled: tables, fields, pages, enums, l
| [AC0030](ac0030/) | Use return value for better error handling | Info | ✓ | |
| [AC0031](ac0031/) | Table data access requires explicit object permissions | Info | ✓ | ✓ |
| [AC0032](ac0032/) | Unused permission declared | Info | ✓ | ✓ |
| [AC0033](ac0033/) | Rest Client requires an Http Client Handler implementation | Warning | ✓ | |
| [AC0034](ac0034/) | Telemetry requires a Telemetry Logger implementation | Warning | ✓ | |

**Note:** Rules marked with "—" in the Enabled column are disabled by default and must be explicitly enabled in your project's ruleset file.
Loading