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

QueuedRetry removal, metrics around spans and services #1910

Closed
ibawt opened this issue Oct 6, 2020 · 9 comments
Closed

QueuedRetry removal, metrics around spans and services #1910

ibawt opened this issue Oct 6, 2020 · 9 comments
Assignees
Labels
bug Something isn't working priority:p3 Lowest

Comments

@ibawt
Copy link
Contributor

ibawt commented Oct 6, 2020

with the queued retry removal / deprecation I lost metrics around the spans accepted and the service it was.

I like to use this as an overall indicator of who is producing what and to what cluster.
Not sure if it should be post / pre sampling, I think it would be fine either as a processor or integrated into exporterhelper

eg

for each resource span bundle {
   inc( span_count, service_name )
}
@ibawt ibawt added the bug Something isn't working label Oct 6, 2020
@bogdandrutu
Copy link
Member

I would suggest having a "TelemetryProcessor" to expose this internal metrics. The main reason:

  • Avoid current duplication - few metrics like this are duplicated.
  • Allow users to install this anywhere in the pipeline - before sampling, after sampling, etc.
  • Allow customization.

@bogdandrutu
Copy link
Member

@ibawt are you willing to help with this issue?

@ibawt
Copy link
Contributor Author

ibawt commented Oct 10, 2020

Either me or a new team member, I guess you can assign to me.

@bogdandrutu
Copy link
Member

Suggest to start from the current queued_retry processor and copy only the telemetry recording part.

@flands
Copy link
Contributor

flands commented Nov 12, 2020

Hey folks -- any updates on this? I would like to argue this is a significant regression and something that needs to be addressed ASAP. From what I can tell, if you have an exporter destination that returns 500 or even 429 status code you basically have no visibility. Metrics look good, zpages look good, the only clue you get is a log message which does not align with metric and zpages data:

2020-11-12T02:54:40.402Z	ERROR	exporterhelper/queued_retry.go:149	Dropping data because sending_queue is full. Try increasing queue_size.	{"component_kind": "exporter", "component_type": "sapm", "component_name": "sapm", "dropped_items": 1

@dmitryax
Copy link
Member

Hi @ibawt , do you still plan to resume your work on this? Otherwise can take it over

@ibawt
Copy link
Contributor Author

ibawt commented Mar 22, 2021

not doing anything related to this anymore cheers. My old company has a version of it running production for a while now but it's so trivial there's no point to code sharing.

@ibawt ibawt removed their assignment Mar 22, 2021
@dmitryax
Copy link
Member

Thanks @ibawt! I'll work on this issue then

@dmitryax dmitryax self-assigned this Mar 22, 2021
@dmitryax
Copy link
Member

dmitryax commented Jul 9, 2021

Metrics lost due to removed queued_retry processor are restored in these PRs:

The following metrics added:

  • exporter/queue_size
  • exporter/enqueue_failed_metric_points
  • exporter/enqueue_failed_spans
  • exporter/enqueue_failed_log_records

@dmitryax dmitryax closed this as completed Jul 9, 2021
hughesjj pushed a commit to hughesjj/opentelemetry-collector that referenced this issue Apr 27, 2023
Bumps [go.uber.org/zap](https://github.com/uber-go/zap) from 1.22.0 to 1.23.0.
- [Release notes](https://github.com/uber-go/zap/releases)
- [Changelog](https://github.com/uber-go/zap/blob/master/CHANGELOG.md)
- [Commits](uber-go/zap@v1.22.0...v1.23.0)

---
updated-dependencies:
- dependency-name: go.uber.org/zap
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working priority:p3 Lowest
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants