Skip to content

Conversation

@rjenkins
Copy link
Contributor

Completes: STR-3494

This PR brings rotel-nodejs up to the latest release of rotel and adds support for the Datadog, Kafka, Blackhole, and Clickhouse exporters. Additionally we've modified the configuration to support multiple exporters. We've tested locally by building the latest main branch of rotel and swapping out the rotel-agent binary in the node_modules/@streamfold/bin directory. Additionally we've tested all exporters with the rotel-nodejs-hello-world project, linking to this branch and changes on the local filesystem via modifying package.json like so.

 "dependencies": {                                                                                                                                                                                                        
│ 14   │ "@streamfold/rotel": "file:../../../rotel-nodejs/npm/app",           

We should follow up with more comprehensive unit testing in additional PRs.

@rjenkins rjenkins requested a review from mheffner July 28, 2025 16:28
Copy link
Contributor

@mheffner mheffner left a comment

Choose a reason for hiding this comment

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

looks good, mostly would recommend bumping to v23 (I'm about to do same for pyrotel)

static kafka_exporter(config?: Partial<KafkaExporter>): KafkaExporter {
return {
_type: "kafka",
security_protocol: "plaintext",
Copy link
Contributor

Choose a reason for hiding this comment

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

Curious why this has to be specified?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Got an error that it was unspecified and needed to be from rdkafka iirc, suspect it was related to the command line arg changes

Copy link
Contributor

Choose a reason for hiding this comment

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

I believe that is now fixed in the latest release.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's what I was thinking

@rjenkins rjenkins merged commit 1371a83 into main Jul 28, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants