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

Span Attributes with Double and Int Values Incorrectly Added to Measurements in Azure Monitor Schema #29683

Closed
rajkumar-rangaraj opened this issue Dec 7, 2023 · 6 comments
Labels
bug Something isn't working exporter/azuremonitor needs triage New item requiring triage

Comments

@rajkumar-rangaraj
Copy link
Contributor

rajkumar-rangaraj commented Dec 7, 2023

Component(s)

exporter/azuremonitor

What happened?

Description

There is a bug in the Azure Monitor exporter where span attributes of type double and int are being added to the measurements field of the Application Insights schema, rather than the properties field. According to the Application Insights data model, the measurements field is intended primarily for timeSinceEnqueued and some specific values, not general span attributes.

Steps to Reproduce

Here is an example of the current behavior, where span attributes like http.response.status_code and server.port are incorrectly placed in the measurements field:

{
	"ver": 1,
	"name": "Microsoft.ApplicationInsights.Request",
	"data": {
		"baseType": "RequestData",
		"baseData": {
			"ver": 2,
			"id": "48c3990c508a86a0",
			"source": "",
			"name": "GET",
			"properties": {
				"http.request.method": "GET",
				"network.protocol.version": "2",
				"url.path": "/favicon.ico",
				"url.scheme": "https",
				"user_agent.original": "Mozilla/5.0 ..."
			},
			"measurements": {
				"http.response.status_code": 404,
				"server.port": 5001
			}
		}
	}
}

Expected Result

Span attributes, especially those of double and int types, should be added to the properties field to align with the Application Insights schema and ensure correct data representation and querying capabilities.

Actual Result

{
	"ver": 1,
	"name": "Microsoft.ApplicationInsights.Request",
	"data": {
		"baseType": "RequestData",
		"baseData": {
			"ver": 2,
			"id": "48c3990c508a86a0",
			"source": "",
			"name": "GET",
			"properties": {
				"http.request.method": "GET",
				"network.protocol.version": "2",
				"url.path": "/favicon.ico",
				"url.scheme": "https",
				"user_agent.original": "Mozilla/5.0 ...",
				"http.response.status_code": "404",
				"server.port": "5001"
			},
			"measurements": {
			}
		}
	}
}

Suggested Fix

Modify the exporter logic to ensure that span attributes of type double and int are correctly added to the properties field instead of measurements.

Collector version

0.90.1

@rajkumar-rangaraj rajkumar-rangaraj added bug Something isn't working needs triage New item requiring triage labels Dec 7, 2023
Copy link
Contributor

github-actions bot commented Dec 7, 2023

Pinging code owners:

See Adding Labels via Comments if you do not have permissions to add labels yourself.

@crobert-1
Copy link
Member

Hello @rajkumar-rangaraj, thanks for filing this issue! I'm pretty unfamiliar with AppInsights and its semantic conventions for traces, but I'm not sure I agree (or I might just be misunderstanding). I might be misinterpreting things here though, so please feel free to let me know! (I might be misinterpreting measurements vs. custom measurements, I don't know)

Measurements
From other documentation it looks like measurements are used for any relevant measurement to the telemetry that will be a numeric value.

Collection of custom measurements: Use this collection to report named measurements associated with the telemetry item. Typical use cases are:

- The size of the dependency telemetry payload.
- The number of queue items processed by request telemetry.
- The time that a customer took to finish the wizard step completing event telemetry.

Also:

Don't use string values for custom measurements. Only numeric values are supported.

Properties
Properties are described there as well, but I also found more information here.

Name-value collection of custom properties: This collection is used to extend standard telemetry with the custom dimensions. Examples are the deployment slot that produced telemetry or the telemetry-item specific property like the order number.

Also:

Properties are string values that you can use to filter your telemetry in the usage reports.

@rajkumar-rangaraj
Copy link
Contributor Author

@crobert-1 CustomEvents are not yet supported through the Azure Monitor Exporter; they remain a feature of the classic Application Insights and are supported only when used with the classic Application Insights SDK. The article and information you provided are based on CustomEvents.

In Application Insights, traces are converted into request and dependency telemetry. This bug highlights an issue where float and int attributes were incorrectly added to the measurement fields of Request/Dependency telemetry. These telemetry types are not designed to accommodate such measurements, and as a result, the values are dropped at the service level.

The Azure Monitor Exporter for .NET is now generally available (GA), and a solution can be derived from it. You can find more information here: Azure SDK GitHub Repository. I have created a pull request to address this issue

@crobert-1
Copy link
Member

Okay, my lack of understanding is showing through here, thanks for clarifying that the documentation I found was for something else.

I saw the GitHub link you shared earlier, but I was hoping there would be more documentation that's relevant to this situation, specifically what measurements mean in this context. It's hard to make a judgement based on a single usage, if that's fair. Do you know if there's any other definitive documentation available on what should be properties vs measurements?

@rajkumar-rangaraj
Copy link
Contributor Author

No additional documentation, apart from the REST API specifications available at https://github.com/Azure/azure-rest-api-specs/blob/main/specification/applicationinsights/data-plane/Monitor.Exporters/preview/v2.1/swagger.json, and the Azure Monitor Exporters implementation for all other languages, including .NET, Java, Node.js, and Python.

@crobert-1
Copy link
Member

I'll let others chime in, sorry for taking so much of your time here. I wanted to make sure the change was the right thing to do, since it might be a breaking change to end-users who expect data in a certain format. Thanks for providing all of the links and references!

djaglowski pushed a commit that referenced this issue Jan 10, 2024
…AzureMonitorExporter (#29684)

**Description:** <Describe what has changed.>
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->

This pull request addresses an issue in the Azure Monitor exporter where
span attributes with double and int values were incorrectly added to the
`measurements` field instead of the `properties` field in the
Application Insights schema.

**Changes**

- Modified the AzureMonitorExporter to ensure that span attributes of
type double and int are correctly placed in the properties field.
- Manual testing was conducted to verify that the span attributes appear
correctly in the properties field.

**Link to tracking Issue:** <Issue number if applicable> #29683 

**Testing:** <Describe what testing was performed and which tests were
added.>

- Updated relevant tests to reflect this change and ensure correctness.
cparkins pushed a commit to AmadeusITGroup/opentelemetry-collector-contrib that referenced this issue Jan 10, 2024
…AzureMonitorExporter (open-telemetry#29684)

**Description:** <Describe what has changed.>
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->

This pull request addresses an issue in the Azure Monitor exporter where
span attributes with double and int values were incorrectly added to the
`measurements` field instead of the `properties` field in the
Application Insights schema.

**Changes**

- Modified the AzureMonitorExporter to ensure that span attributes of
type double and int are correctly placed in the properties field.
- Manual testing was conducted to verify that the span attributes appear
correctly in the properties field.

**Link to tracking Issue:** <Issue number if applicable> open-telemetry#29683 

**Testing:** <Describe what testing was performed and which tests were
added.>

- Updated relevant tests to reflect this change and ensure correctness.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working exporter/azuremonitor needs triage New item requiring triage
Projects
None yet
Development

No branches or pull requests

2 participants