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

New component: Named Pipe Receiver #27234

Closed
2 tasks
sinkingpoint opened this issue Sep 27, 2023 · 8 comments
Closed
2 tasks

New component: Named Pipe Receiver #27234

sinkingpoint opened this issue Sep 27, 2023 · 8 comments
Labels
Accepted Component New component has been sponsored Stale

Comments

@sinkingpoint
Copy link
Contributor

sinkingpoint commented Sep 27, 2023

The purpose and use-cases of the new component

It's sometimes useful to be able to sends telemetry to a collector running on a node over a named pipe, rather than having to deal with the overhead of network protocols, or the inherent disk buffering of writing to a file. I'd like to propose a receiver that has the ability to create a named pipe, and then read logs off of it, similar to https://github.com/open-telemetry/opentelemetry-collector-contrib/tree/main/receiver/tcplogreceiver or https://github.com/open-telemetry/opentelemetry-collector-contrib/tree/main/receiver/filereceiver

Example configuration for the component

receivers:
  pipes:
    - path: /pipes/logs1
    - path: /pipes/logs2
      mode: 0600
  encoding: utf-8
  multiline: # A split.Config
    line_end_pattern: "\n"
  preserve_trailing_whitespaces: false # A trim.Config

I suspect the complete config would end up looking something like the filereceiver one, with multiline config etc.

Telemetry data types supported

Logs

Is this a vendor-specific component?

  • This is a vendor-specific component
  • If this is a vendor-specific component, I am proposing to contribute and support it as a representative of the vendor.

Code Owner(s)

@sinkingpoint

Sponsor (optional)

@djaglowski

Additional context

No response

@bryan-aguilar
Copy link
Contributor

Hey, @sinkingpoint

Thank you for the new component proposal. If you have not already please make sure you review the new component guidelines.

If you have not found a volunteer sponsor yet then I encourage you to come to our weekly collector sig meetings. You can add an item to the agenda to discuss this new component proposal.

@bryan-aguilar bryan-aguilar removed the needs triage New item requiring triage label Oct 13, 2023
@atoulme
Copy link
Contributor

atoulme commented Oct 18, 2023

We discussed during the APAC SIG - idea looks good, no objections. @djaglowski it seems this would build best on stanza, do you see any trouble with this approach? Would you be interested to sponsor perhaps?

@djaglowski
Copy link
Member

I think it makes a lot of sense as a stanza input operator, which is then wrapped into a receiver. I'm willing to sponsor this.

@sinkingpoint, are you good with first writing the stanza input operator only? From there it should be fairly straightforward to create the receiver.

@sinkingpoint
Copy link
Contributor Author

Thanks @djaglowski , appreciate it. Happy to write the stanza operator - I'll work on it next week

@sinkingpoint
Copy link
Contributor Author

Looking around the code, would it make sense to follow the same pattern as the file receiver and further abstract the logic into a namedpipeconsumer (or adapt the existing fileconsumer), or keep it self contained as an input module?

@djaglowski
Copy link
Member

I definitely don't think we should add anything to fileconsumer package, as it's already complicated enough trying to do one thing. There may be some functionality we need to pull out of there and share, but I'm not convinced this is the case.

I think it probably makes sense to first try a full implementation within pkg/stanza/operator/input/namedpipe. If there's a specific reason we can't do a fully separate implementation there (e.g. duplicating complex code), let's talk about those details.

Off the top of my head, I think the biggest area of duplication is going to be in splitting, trimming, flushing, etc. However, all of this functionality is already separated into distinct packages which can be composed as needed. I think there may be a little bit of copying the way these are configured and the order of composition from fileconsumer, but ultimately I'm not convinced this functionality should be packed into a monolithic package, since other use cases only need a subset of it.

@crobert-1 crobert-1 added Accepted Component New component has been sponsored and removed Sponsor Needed New component seeking sponsor labels Nov 1, 2023
djaglowski pushed a commit that referenced this issue Nov 15, 2023
**Description:** 

This is a namedpipe input operator, which will read from a named pipe
and send the data to the pipeline. It pretty closely mimics the file
input operator, but with a few differences.

In particular, named pipes have an interesting property that they
receive EOFs when a writer closes the pipe, but that _doesn't_ mean that
the pipe is closed. To solve this issue, we crib from existing `tail -f`
implementations and use an inotify watcher to detect whenever the pipe
receives new data, and then read it using the standard `bufio.Scanner`
reader.

**Link to tracking Issue:** #27234

**Testing:**

We add a couple of tests for the new operator. The first tests simply
the creation of the named pipe - checking that it's created as a pipe,
with the right permissions. The second goes further by inserting logs
over several different `Open`s into the pipe, testing that the logs are
read, and that the operator is able to handle the named pipe behavior of
skipping over EOFs.

**Documentation:**

None, at the moment

/cc @djaglowski

---------

Signed-off-by: sinkingpoint <colin@quirl.co.nz>
RoryCrispin pushed a commit to ClickHouse/opentelemetry-collector-contrib that referenced this issue Nov 24, 2023
**Description:** 

This is a namedpipe input operator, which will read from a named pipe
and send the data to the pipeline. It pretty closely mimics the file
input operator, but with a few differences.

In particular, named pipes have an interesting property that they
receive EOFs when a writer closes the pipe, but that _doesn't_ mean that
the pipe is closed. To solve this issue, we crib from existing `tail -f`
implementations and use an inotify watcher to detect whenever the pipe
receives new data, and then read it using the standard `bufio.Scanner`
reader.

**Link to tracking Issue:** open-telemetry#27234

**Testing:**

We add a couple of tests for the new operator. The first tests simply
the creation of the named pipe - checking that it's created as a pipe,
with the right permissions. The second goes further by inserting logs
over several different `Open`s into the pipe, testing that the logs are
read, and that the operator is able to handle the named pipe behavior of
skipping over EOFs.

**Documentation:**

None, at the moment

/cc @djaglowski

---------

Signed-off-by: sinkingpoint <colin@quirl.co.nz>
djaglowski pushed a commit that referenced this issue Dec 13, 2023
**Description:**

This adds a new receiver that reads from a named pipe, using the
previously created namedpipeinput stanza operator (#28841). Because that
operator does the majority of the work, this is mostly boilerplate + a
few tests to confirm that it works e2e.

Link to tracking Issue: #27234

**Testing:**

Added a few tests, loading the config, and actually creating a receiver
to verify that it can read logs from the pipe.

**Documentation:**

None, yet.

/cc @djaglowski - where should I add docs for this if any?
nslaughter pushed a commit to nslaughter/opentelemetry-collector-contrib that referenced this issue Dec 27, 2023
**Description:**

This adds a new receiver that reads from a named pipe, using the
previously created namedpipeinput stanza operator (open-telemetry#28841). Because that
operator does the majority of the work, this is mostly boilerplate + a
few tests to confirm that it works e2e.

Link to tracking Issue: open-telemetry#27234

**Testing:**

Added a few tests, loading the config, and actually creating a receiver
to verify that it can read logs from the pipe.

**Documentation:**

None, yet.

/cc @djaglowski - where should I add docs for this if any?
nslaughter pushed a commit to nslaughter/opentelemetry-collector-contrib that referenced this issue Dec 27, 2023
**Description:**

This adds a new receiver that reads from a named pipe, using the
previously created namedpipeinput stanza operator (open-telemetry#28841). Because that
operator does the majority of the work, this is mostly boilerplate + a
few tests to confirm that it works e2e.

Link to tracking Issue: open-telemetry#27234

**Testing:**

Added a few tests, loading the config, and actually creating a receiver
to verify that it can read logs from the pipe.

**Documentation:**

None, yet.

/cc @djaglowski - where should I add docs for this if any?
cparkins pushed a commit to AmadeusITGroup/opentelemetry-collector-contrib that referenced this issue Jan 10, 2024
**Description:**

This adds a new receiver that reads from a named pipe, using the
previously created namedpipeinput stanza operator (open-telemetry#28841). Because that
operator does the majority of the work, this is mostly boilerplate + a
few tests to confirm that it works e2e.

Link to tracking Issue: open-telemetry#27234

**Testing:**

Added a few tests, loading the config, and actually creating a receiver
to verify that it can read logs from the pipe.

**Documentation:**

None, yet.

/cc @djaglowski - where should I add docs for this if any?
Copy link
Contributor

This issue has been inactive for 60 days. It will be closed in 60 days if there is no activity. To ping code owners by adding a component label, see Adding Labels via Comments, or if you are unsure of which component this issue relates to, please ping @open-telemetry/collector-contrib-triagers. If this issue is still relevant, please ping the code owners or leave a comment explaining why it is still relevant. Otherwise, please close it.

@github-actions github-actions bot added the Stale label Jan 29, 2024
@djaglowski
Copy link
Member

Closed by #29320

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Accepted Component New component has been sponsored Stale
Projects
None yet
Development

No branches or pull requests

5 participants