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

[chore][exporter/azuremonitor] Force the telemetry to be sent to azuremonitor #36172

Conversation

whitneygriffith
Copy link
Contributor

Description

  1. Resolved an issue where traces weren't being sent to App Insights due to not flushing the Telemetry Channel. Added the necessary flush operation to ensure all traces, metrics and logs are properly sent to the queue, leveraging App Insights' batch handling for more efficient processing.

Link to tracking issue

Resolves #35037

Signed-off-by: whitneygriffith <whitney.griffith16@gmail.com>
@andrzej-stencel
Copy link
Member

Sounds like this is a bug fix. Can you add a changelog entry? https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/CONTRIBUTING.md#adding-a-changelog-entry

@whitneygriffith whitneygriffith changed the title [exporter/azuremonitor] Force the telemetry to be sent to azuremonitor [chore exporter/azuremonitor] Force the telemetry to be sent to azuremonitor Nov 6, 2024
@whitneygriffith whitneygriffith changed the title [chore exporter/azuremonitor] Force the telemetry to be sent to azuremonitor [chore][exporter/azuremonitor] Force the telemetry to be sent to azuremonitor Nov 6, 2024
Signed-off-by: whitneygriffith <whitney.griffith16@gmail.com>
@whitneygriffith
Copy link
Contributor Author

Sounds like this is a bug fix. Can you add a changelog entry? https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/CONTRIBUTING.md#adding-a-changelog-entry

I have added the changelog entry.

@andrzej-stencel andrzej-stencel added the ready to merge Code review completed; ready to merge by maintainers label Nov 6, 2024
@andrzej-stencel
Copy link
Member

andrzej-stencel commented Nov 6, 2024

There are some errors here: https://github.com/open-telemetry/opentelemetry-collector-contrib/actions/runs/11700948723/job/32586089416?pr=36172

Error: ./factory_test.go:20:25: cannot use &mockTransportChannel{} (value of type *mockTransportChannel) as transportChannel value in struct literal: *mockTransportChannel does not implement transportChannel (missing method Flush)

@mx-psi mx-psi removed the ready to merge Code review completed; ready to merge by maintainers label Nov 8, 2024
Copy link
Contributor

@fatsheep9146 fatsheep9146 left a comment

Choose a reason for hiding this comment

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

There are still some unresolved failed tests https://github.com/open-telemetry/opentelemetry-collector-contrib/actions/runs/11711127058/job/32641600088?pr=36172

=== FAIL: . TestExporterLogDataCallback (0.00s)
panic: 
	assert: mock: I don't know what to return because the method call was unexpected.
		Either do Mock.On("Flush").Return(...) first, or remove the Flush() call.
		This method was unexpected:
			Flush()
			
		at: [/home/runner/work/opentelemetry-collector-contrib/opentelemetry-collector-contrib/exporter/azuremonitorexporter/mock_transportChannel.go:24 /home/runner/work/opentelemetry-collector-contrib/opentelemetry-collector-contrib/exporter/azuremonitorexporter/logexporter.go:39 /home/runner/work/opentelemetry-collector-contrib/opentelemetry-collector-contrib/exporter/azuremonitorexporter/logexporter_test.go:118] [recovered]
	panic: 
	assert: mock: I don't know what to return because the method call was unexpected.
		Either do Mock.On("Flush").Return(...) first, or remove the Flush() call.
		This method was unexpected:
			Flush()
			
		at: [/home/runner/work/opentelemetry-collector-contrib/opentelemetry-collector-contrib/exporter/azuremonitorexporter/mock_transportChannel.go:24 /home/runner/work/opentelemetry-collector-contrib/opentelemetry-collector-contrib/exporter/azuremonitorexporter/logexporter.go:39 /home/runner/work/opentelemetry-collector-contrib/opentelemetry-collector-contrib/exporter/azuremonitorexporter/logexporter_test.go:118]

Copy link
Contributor

@fatsheep9146 fatsheep9146 left a comment

Choose a reason for hiding this comment

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

I think you should do as this lint said, could you please fix this? https://github.com/open-telemetry/opentelemetry-collector-contrib/actions/runs/11904647742/job/33173837807?pr=36172

Generated code is out of date, please run "make generate" and commit the changes in this PR.

Signed-off-by: whitneygriffith <whitney.griffith16@gmail.com>
@fatsheep9146 fatsheep9146 added the ready to merge Code review completed; ready to merge by maintainers label Nov 21, 2024
@codeboten
Copy link
Contributor

Closing and re-opening as it looks like the CI checks are stuck.

@codeboten codeboten closed this Nov 22, 2024
@codeboten
Copy link
Contributor

Apparently I cannot reopen it :(

Screenshot 2024-11-22 at 2 52 22 PM

@fatsheep9146
Copy link
Contributor

@whitneygriffith How about you create a same pr again? It seems that this pr is not able to be merged due to the stuck CI.

@whitneygriffith
Copy link
Contributor Author

Created #36520

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
exporter/azuremonitor ready to merge Code review completed; ready to merge by maintainers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Istio Traces Aren't Being Exported with Azure Monitor Exporter
6 participants