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.

Sign up for GitHub

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

Jump to bottom

Add e2e pipeline processing duration self-observability metrics #3183

Closed
wants to merge 2 commits into from
Closed

Add e2e pipeline processing duration self-observability metrics #3183

wants to merge 2 commits into from

Conversation

dashpole
Copy link
Contributor

@dashpole dashpole commented May 14, 2021 •
edited
Loading

Description:
Add a self-observability metric that measures the time between when a batch of telemetry is received by a receiver to when it is exported by an exporter.

Notes to reviewers:

  • obsreport/obsreport_consumer.go adds the pipeline tag to pipeline metrics. Individual components (receivers, exporters) can be shared between multiple pipelines, so I can't do tagging there, and had to add a per-pipeline consumer to do the tagging.
  • This adds an exported function MergeContexts to the obsreport package to handle cases (such as batching) in which there are multiple input contexts for each output context.

Since we were still uncertain if this is the right approach at the last sig meeting, i'm going to wait for the go-ahead from @bogdandrutu or @tigrannajaryan to add testing and documentation.

Link to tracking Issue: #542

Testing: TODO. Looking for feedback on the general approach first.

Documentation: TODO. Looking for feedback on the general approach first.

@dashpole dashpole force-pushed the context_latency_e2e branch from 169a390 to 4ed3e0d Compare May 14, 2021 15:53
@dashpole dashpole mentioned this pull request May 14, 2021
Open
@dashpole dashpole force-pushed the context_latency_e2e branch 2 times, most recently from 8b4d99d to 57391c1 Compare May 14, 2021 16:07
@alolita alolita requested a review from bogdandrutu May 18, 2021 06:40
@tigrannajaryan tigrannajaryan self-assigned this May 19, 2021
MovieStoreGuy
MovieStoreGuy reviewed May 20, 2021
View reviewed changes
Copy link
Contributor

@MovieStoreGuy MovieStoreGuy left a comment

Choose a reason for hiding this comment

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

I've added my unasked for review (but since I am also interested in this), I have added my 2c to at least get the conversation started :)

obsreport/obsreport_pipeline.go Outdated Show resolved Hide resolved
obsreport/obsreport_pipeline.go Outdated Show resolved Hide resolved
obsreport/obsreport_pipeline.go Show resolved Hide resolved
processor/batchprocessor/batch_processor.go Show resolved Hide resolved
@jrcamp jrcamp mentioned this pull request May 21, 2021
Closed
@dashpole dashpole force-pushed the context_latency_e2e branch from 57391c1 to d8b5067 Compare May 21, 2021 18:08
@github-actions
Copy link
Contributor

This PR was marked stale due to lack of activity. It will be closed in 7 days.

@github-actions github-actions bot added the Stale label May 29, 2021
@dashpole dashpole force-pushed the context_latency_e2e branch from d8b5067 to 99b2a96 Compare June 3, 2021 20:54
@dashpole
Copy link
Contributor Author

dashpole commented Jun 3, 2021

I rebased this, and am still hoping to see if the approach is acceptable. Let me know if there is anything I can do to help, or other approaches I should try.

@github-actions github-actions bot removed the Stale label Jun 4, 2021
@dashpole dashpole force-pushed the context_latency_e2e branch 2 times, most recently from 7e69a99 to 28a2eb2 Compare June 7, 2021 13:20
@github-actions
Copy link
Contributor

This PR was marked stale due to lack of activity. It will be closed in 7 days.

@github-actions github-actions bot added the Stale label Jun 15, 2021
@alolita alolita removed the Stale label Jun 15, 2021
@alolita
Copy link
Member

alolita commented Jun 15, 2021

@open-telemetry/collector-approvers can you please review this PR? cc @tigrannajaryan @bogdandrutu

@github-actions
Copy link
Contributor

This PR was marked stale due to lack of activity. It will be closed in 7 days.

@github-actions github-actions bot added the Stale label Jun 23, 2021
@dashpole dashpole force-pushed the context_latency_e2e branch from 28a2eb2 to c267663 Compare June 23, 2021 16:03
@dashpole dashpole marked this pull request as ready for review June 23, 2021 16:07
@dashpole dashpole requested a review from a team June 23, 2021 16:07
@dashpole dashpole force-pushed the context_latency_e2e branch from 44c9dba to 9cb8d28 Compare June 23, 2021 20:46
@bogdandrutu bogdandrutu removed the Stale label Jun 28, 2021
@github-actions
Copy link
Contributor

github-actions bot commented Jul 6, 2021

This PR was marked stale due to lack of activity. It will be closed in 7 days.

@dashpole dashpole force-pushed the context_latency_e2e branch from 9cb8d28 to 8662996 Compare July 7, 2021 16:38
@dashpole
Copy link
Contributor Author

dashpole commented Jul 7, 2021

rebased again. Still looking for feedback

@github-actions github-actions bot removed the Stale label Jul 8, 2021
@dashpole
Copy link
Contributor Author

@tigrannajaryan should I follow the pattern in #3552 once it merges, rather than use context?

@tigrannajaryan
Copy link
Member

@tigrannajaryan should I follow the pattern in #3552 once it merges, rather than use context?

I think PdataContext that I suggested should work nicely for keeping the start time of pdata and calculating the duration at the end.

@tigrannajaryan
Copy link
Member

@dashpole do you want to keep the PR open or maybe convert to draft until we have PdataContext support?

@dashpole dashpole marked this pull request as draft August 9, 2021 12:57
@dashpole
Copy link
Contributor Author

dashpole commented Aug 9, 2021

That makes sense. Converted to a draft

@github-actions
Copy link
Contributor

This PR was marked stale due to lack of activity. It will be closed in 7 days.

@github-actions github-actions bot added the Stale label Aug 17, 2021
@bogdandrutu bogdandrutu removed the Stale label Aug 17, 2021
@github-actions
Copy link
Contributor

This PR was marked stale due to lack of activity. It will be closed in 7 days.

@github-actions github-actions bot added the Stale label Aug 25, 2021
@bogdandrutu bogdandrutu removed the Stale label Aug 25, 2021
@github-actions
Copy link
Contributor

github-actions bot commented Sep 2, 2021

This PR was marked stale due to lack of activity. It will be closed in 7 days.

@github-actions github-actions bot added the Stale label Sep 2, 2021
@bogdandrutu bogdandrutu removed the Stale label Sep 3, 2021
@github-actions
Copy link
Contributor

This PR was marked stale due to lack of activity. It will be closed in 7 days.

@github-actions github-actions bot added the Stale label Sep 11, 2021
@bogdandrutu bogdandrutu removed the Stale label Sep 13, 2021
@github-actions
Copy link
Contributor

This PR was marked stale due to lack of activity. It will be closed in 7 days.

@github-actions github-actions bot added the Stale label Sep 22, 2021
@bogdandrutu bogdandrutu removed the Stale label Sep 22, 2021
@github-actions
Copy link
Contributor

This PR was marked stale due to lack of activity. It will be closed in 7 days.

@github-actions github-actions bot added the Stale label Sep 30, 2021
@github-actions
Copy link
Contributor

github-actions bot commented Oct 7, 2021

Closed as inactive. Feel free to reopen if this PR is still being worked on.

@github-actions github-actions bot closed this Oct 7, 2021
@peachchen0716
Copy link

Hi @tigrannajaryan I am looking for this e2e metrics too. What's status for PdataContext? Thanks!

@tigrannajaryan
Copy link
Member

Hi @tigrannajaryan I am looking for this e2e metrics too. What's status for PdataContext? Thanks!

@peachchen0716 I no longer work on the Collector so can't answer this. I suggest to create a new issue and tag current maintainers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MovieStoreGuy MovieStoreGuy MovieStoreGuy left review comments

@bogdandrutu bogdandrutu Awaiting requested review from bogdandrutu

Assignees

@tigrannajaryan tigrannajaryan

Labels
Stale
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@dashpole @alolita @tigrannajaryan @peachchen0716 @MovieStoreGuy @bogdandrutu