Skip to content

Add the missing usage report keys for the primary instance, its storage and its transport - #5945

Open
johnsimons wants to merge 1 commit into
masterfrom
john/missing_telemetry
Open

johnsimons wants to merge 1 commit into
masterfrom
john/missing_telemetry

Conversation

@johnsimons

Copy link
Copy Markdown
Member

This PR just adds all the missing settings that the user can customise.

… the usage report

Expands the environment data collected for the usage report to cover transport authentication and options (ASBS, SQS, RabbitMQ, SQL Server, PostgreSQL), storage authentication and tuning (RavenDB, SQL Server, PostgreSQL, EF Core), and error instance settings (ingestion tuning, heartbeats, recoverability, security posture, ServicePulse integration, logging, and OTLP metrics).

Values are reported as fixed labels rather than raw configuration values, so customer-specific details (queue names, endpoints, secrets, hostnames, claim names) are never exposed.

@warwickschroeder warwickschroeder left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good. Confirmed no real data is being collected, only fixed words to indicate use.

I noticed that some transports dont have overrides - IBM MQ, ASQ, MSMQ. Looks like ASQ is feature complete, and MSMQ is suset so thats probably why. IBM MQ is active though.


protected virtual string GetAuthenticationMode(TransportSettings transportSettings) => "NotApplicable";

protected virtual bool RelaxesCertificateValidation(TransportSettings transportSettings) => false;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks like only Rabbit overrides this? SqlServerTransportCustomization could override this to look for TrustServerCertificate=True, and postgres to look for SslMode=Require. I think the other transports probably also have a ssl switch that could be parsed potentially.

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.

3 participants