New otelcol.exporter.debug component

[BarunKGP]

PR Description

Added a new otelcol.exporter.debug component based on the otelcol debugexporter.

Which issue(s) this PR fixes

Fixes grafana/alloy#301

Notes to the Reviewer

• Based on same design as other otelcol.exporter components, specifically the otelcol.exporter.logging component (as debug exporter is replacing logging exporter).
• Uses the same defaults as specified in the otelcol debugexporter factory
• Added exporterArguments type which is a wrapper over Arguments and maintains an implementation that is consistent with debugexporter.
  • Removed DebugMetrics field from Arguments struct. However, it couldn't be completely removed because the upstream exporter.Argument specifies this in its interface.
  • Arguments.Verbosity is now a string and can be set to a string (such as "detailed"). exporterArguments deals with the conversion into configtelemetry.Level type
• Added tests to check that component Config is created from Arguments and error is thrown when an invalid Verbosity is passed

PR Checklist

• CHANGELOG.md updated
• Tests added

[ptodev]

Thank you for your contribution! I left a few comments. Would you mind also adding a couple of things please:

• Unit tests - they need to be in a debug_test.go file, similar to spanmetrics_test.go.
• A documentation page - it could be similar to otelcol.exporter.logging.md

[BarunKGP]

Thank you for the review and suggestions @ptodev!

Based on your comments, I felt that having an internal exporterArguments type to maintain interoperability with the upstream exporter should help us get the functionality we need. This keeps the main Arguments type clean of unwanted fields (such as DebugMetrics) and lets us control the types of the required fields, while maintaining easy interoperability with otelcol's exporter. I've also modified the Convert() function that is part of the Arguments interface to first convert an Argument type to exporterArgument and use that to generate the config.

I've updated the PR with the following changes:

• Added exporterArguments type which is a wrapper over Arguments and maintains an implementation that is consistent with debugexporter.
  • Removed DebugMetrics field from Arguments struct. However, it couldn't be completely removed because the upstream exporter.Argument specifies this in its interface.
  • Arguments.Verbosity is now a string and can be set to a string (such as "detailed"). exporterArguments deals with the conversion into configtelemetry.Level type
• Added tests to check that component Config is created from Arguments and error is thrown when an invalid Verbosity is passed
• Reverted versions of dependencies in go.mod

Let me know if there's anything else you'd like me to change here.

I'm still working on the documentation part. I'll add it within the next couple of days, if that's okay?

[ptodev]

Hi, thank you for the update! I don't think it's necessary to have a exporterArguments though. It makes adding new arguments more inconvenient, since then we have to add them to both exporterArguments and Arguments it also makes it less clear how default value are applied, and could make the component a bit more slow due to extra copying.

I'll add it within the next couple of days, if that's okay?

Yes, of course, thank you!

[BarunKGP]

@ptodev happy new year! Apologies for the delay in making the requested changes, I got caught up in other stuff over the holidays.

I've now addressed all the review comments you left. In summary:

• Reset go.sum and reverted to the previous dependency versions
• Added tests to debug_test.go in table format and checked that they are passing
• Removed unnecessary exporterArguments field
• Edited changelog for clarity

Please let me know if there are any other changes you'd like me to make. Thanks!

[ptodev]

Hi @BarunKGP! Happy new year, and apologies for the late reply! The code looks great, thank you!

The only remaining task is to add a docs page similar to otelcol.exporter.logging.md. Would you be able to add it please?

PS: There is a minor linter issue too.

[BarunKGP]

@ptodev I've created the doc, but what about the aliases section as in otelcol.exporter.logging.md? Should I remove it, or do we have any new aliases to include here?

Also taking a look at the linter issue. I'll push my changes by this weekend.

[ptodev]

@ptodev I've created the doc, but what about the aliases section as in otelcol.exporter.logging.md? Should I remove it, or do we have any new aliases to include here?

Thank you, the docs look good! It is best to retain the same aliases as otelcol.exporter.logging, so the doc is good as it is.

Also taking a look at the linter issue. I'll push my changes by this weekend.

Thank you! I'll give it a final stamp of approval afterwards. It would be nice to rebase the branch with main too.

[clayton-cornell]

Some doc input.

[CLAassistant]

All committers have signed the CLA.

[BarunKGP]

@clayton-cornell Thank you for the doc suggestions. I have accepted and incorporated all of them.

@ptodev rebased the branch. Let me know if there's anything else I should take a look at :)

[ptodev]

@BarunKGP - The rebase must have gone wrong, because there are lots of changes in the PR now.

[BarunKGP]

@ptodev I've reverted this branch to its state prior to the rebase and added the doc changes to it. Upon investigating, it looks like I was incorporating suggestions from these comments onto my origin/main directly, which is why the two branches diverged.

I'm thinking of creating a new branch with all my changes from here, while I force pull upstream/main on this one and then rebase my changes on top of it. Unfortunately, the git log will get a bit messy. Do you have any suggestions on how to go about this?

[ptodev]

This is the way I normally rebase my feature branches:

git fetch
git rebase origin/main

If you'd like to clean up your branch in a more step-by-step basis, you can also do something like:

# Remove your commits but retain your changes
git reset --soft <comit-before-your-commits>`

git stash

# Make your branch the same as main
# I'm not too sure about this one, but I think it'd work?
git reset --hard main

git stash pop

You could also just create a new branch off of main, apply your changes there, and then use git reset on your old branch to make it point to the same commit as the new one.

[BarunKGP]

I've added all the changes to the branch after rebasing. I've also updated the new components to the revised API for components.

What should be the Stability for this however? I've left it as StabilityExperimental for now.

[ptodev]

Thank you for the update!

What should be the Stability for this however? I've left it as StabilityExperimental for now.

"Experimental" is the right one, yes.

There is a linter error:

internal/component/otelcol/exporter/debug/debug.go:9: File is not `gofmt`-ed with `-s` (gofmt)

I also added a few minor comments. After they are resolved, I think it should be ok for merging.

[BarunKGP]

@ptodev thank you for your time in reviewing the PR!

I have accepted all suggestions and made the following changes addressing them:

• Fixed imports in go.mod -> only added debugexporter as a dependency
• Updated default arguments and test for default arguments
• Fixed linting issues

Please let me know if there are any other areas I need to work on 😃

[ptodev]

Thank you very much! LGTM 🚀

[clayton-cornell]

@ptodev This one seems to have been missed... is it still valid? Does anything need to be moved to Alloy?

[clayton-cornell]

Docs are OK as-is

[clayton-cornell]

@ptodev There is a linting error (outside the docs) that I can't fix. Any ideas?

[ptodev]

@clayton-cornell I fixed it, no worries. Thank you for the review!
@BarunKGP Thank you for your contribution, and apologies for the delay with the review!

I also opened a PR to add the component to Alloy.