- naming:

node_exporter's package is named node-exporter, it doesn't have a prometheus- suffix, so I guess prometheus-snmp-exporter could just be snmp-exporter.

- provides:

upstream package name is snmp_exporter, it would be great to have a "provides" with that name as node-exporter does:

Provides:       snmp_exporter

- generator binary

if the binary will be named snmp_exporter, I think it's safe to name the generator with snmp_generator.

Instead of doing this:

> for cmd in generator; do
>   %gobuild -o %{gobuilddir}/bin/$(basename $cmd) %{goipath}/$cmd
> done

You can do the following and control the generated binary name:

%gobuild -o %{gobuilddir}/bin/snmp_generator %{goipath}/generator

So you would need to adapt these to the new name:

mv %{buildroot}%{_bindir}/generator %{buildroot}%{_bindir}/prometheus-snmp-generator
(...)
%{_bindir}/prometheus-snmp-generator

- systemd 

Needs to depend on systemd-rpm-macros based on https://docs.fedoraproject.org/en-US/packaging-guidelines/Systemd/#packaging

BuildRequires: systemd-rpm-macros

- prometheus user

Service depends on prometheus user, but it's creation is not included in the spec file.

Check how `prometheus` packages does it and https://docs.fedoraproject.org/en-US/packaging-guidelines/UsersAndGroups/ as extra help if needed.

- config file

the service file defines a config file in /etc/prometheus/snmp_exporter/snmp.yml but that's not installed.

I don't know if the sample snmp.yml can be used, but at least the folder needs to be owned by the package.

%dir %{_sysconfdir}/prometheus/snmp_exporter/
%config(noreplace) %{_sysconfdir}/prometheus/snmp_exporter/snmp.yml

About the user, I see `node-exporter` uses also the `prometheus` user, but I'm not sure if all exporters should use same one. Can you try with a new user & group named `snmp-exporter`?

Spec URL: https://www.flyn.org/SRPMS/snmp-exporter.spec
SRPM URL: https://www.flyn.org/SRPMS/snmp-exporter-0.30.1-1.fc44.src.rpm
Description: SNMP Exporter for Prometheus
Fedora Account System Username: mikep

- Rename to match node-exporter
- Add Provides: snmp_exporter
- Use snmp_generator for the name of the generator
- Depend on systemd-rpm-macros
- Create user, and change username to snmp-exporter
- Own config file directory, and create stub

Copr build:
https://copr.fedorainfracloud.org/coprs/build/10714177
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2490148-snmp-exporter/fedora-rawhide-x86_64/10714177-snmp-exporter/fedora-review/review.txt

Found issues:

- Using both %{buildroot} and $RPM_BUILD_ROOT
  Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/#_macros

Please know that there can be false-positives.

---
This comment was created by the fedora-review-service
https://github.com/FrostyX/fedora-review-service

If you want to trigger a new Copr build, add a comment containing new
Spec and SRPM URLs or [fedora-review-service-build] string.

- Using both %{buildroot} and $RPM_BUILD_ROOT

> mkdir -p $RPM_BUILD_ROOT%{_unitdir}
> install -p -m 644 %{SOURCE3} $RPM_BUILD_ROOT%{_unitdir}/

Change to %{buildroot}



With that the spec would be fine, but I've two doubts:

- Service name

Should it be `snmp_exporter.service` or `snmp-exporter.service` (upstream name vs downstream name)?

- etc path

I'm not sure if it should be %{_sysconfdir}/prometheus/snmp_exporter/ or just %{_sysconfdir}/snmp_exporter/


For both items node-exporter uses your approach, but honestly I'm unsure if it's best thing to do. 

I opened https://bugzilla.redhat.com/show_bug.cgi?id=2500076 & https://bugzilla.redhat.com/show_bug.cgi?id=2500075 to import a couple of other exporters and I followed these rules:

- service name is equal to package name, so with "-" instead of "_"
- config file, if any, at %{_sysconfdir}/$name_exporter/
- a environment variable at %{_sysconfdir}/sysconfig/ that is set as "EnvironmentFile=-/etc/sysconfig/blackbox-exporter" in the service file

What do you think about these changes? As I would like to import other exporters (mysql, postgresql, ssl, ...) I'd like to have a defined common pattern.

So far I have tended to follow the lead of node-exporter, but here are my own opinions:

prometheus-snmp-exporter was the better package name. For example, I often run something like "dnf list *prometheus*" to find what is available in package form that is related to prometheus. This would not catch snmp-exporter. Additionally, snmp-exporter is a very generic name. As a point of comparison, lighttpd webserver plugin packages are named, e.g., as lighttpd-mod-openssl.

I am leery of the binaries snmp_exporter and snmp_generator. These are generic names. Having had naming conflicts with other packages, I think adding a prometheus prefix would be better.

I guess I like snmp-exporter.service better.

I do like %{_sysconfdir}/prometheus/snmp_exporter/ better, since snmp_exporter is generally used with prometheus. This keeps /etc a little less cluttered. As a comparison, the lighttpd webserver plugin configurations go in /etc/lighttpd/...

It would be nice to come to a consensus across all exporter packages. Should we start an email discussion among exporter packagers? Or something on Bugzilla? Or something on the fedora-devel mailing list?