Ensure that metric id is always set during kafka ingestion

[fitzoh]

While generating fake metric with parrot for #1680, kafka ingestion was failed due to the metric ID not being set.

This PR ensures that the metric ID is always set, instead of only setting it if autoInterval is true.

[Dieterbe]

looking at tsdbgw, where this code came from, it seems the (undocumented) convention was that input plugins would SetId(), usually very shortly before calling Publish().
(taking the code out of tsdbgw, we can broaden this idea and say that whoever needs to call into Publish() needs to assure the MetricData is valid, has the ID set, etc)
Conceptually this makes sense, since the ID is a property of the MetricData, not just an implementation detail of a publisher.
Looking at the fakemetrics code, it follows the same style. the commands like datafeed etc assure the MetricData is well formed, and call SetId(), before handing off to the output.
(we should end the proliferation of different publisher libraries, and unification of the fakemetrics outputs and the gateway publisher is an important aspect of that)

So why does the kafka publisher in master SetId() [again] when it needs to apply auto-interval detection?
because the interval influences the ID
So why don't we set the interval (and the ID) all in the caller rather than the publisher?
why is the autoInterval even a concept of concern to a publisher? I notice that this feature is controlled by cmd/tsdb-gw/main.go anyway , when it calls kafka.New()
I don't know any good answer to this. It looks like this is simply an abstraction violation that we should fix.

my suggestion is:

1. remove metric.SetId() from the publisher code
2. assure that any caller that uses this publishing code sets the interval properly. (to find such calling code, we only have to look at code within the MT repo I think)

If you need help with this, feel free to hit me up.

[fitzoh]

my suggestion is:

1. remove metric.SetId() from the publisher code
2. assure that any caller that uses this publishing code sets the interval properly. (to find such calling code, we only have to look at code within the MT repo I think)

Done in 9cc9266... The publisher stuff was imported for mt-gateway, and that's the only place it's currently used.

[Dieterbe]

ok but now we still have the bug that if the auto interval stuff is invoked we have the incorrect Id set.
seems like the auto interval stuff should be moved into mt-gateway, out of the kafka publisher.

[fitzoh]

bah, I misread point 2, will do

[fitzoh]

So after doing a bit more digging, mt-gateway is already discarding metrics with an interval of 0.

In prepareIngest we call m.Validate()

metrictank/cmd/mt-gateway/ingest/metrics.go

Lines 86 to 91 in b78a563

	if err := m.Validate(); err != nil {
	log.Debugf("received invalid metric: %v %v %v", m.Name, m.OrgId, m.Tags)
	resp.AddInvalid(err, i)
	promDiscards.Add(m.OrgId, err.Error())
	continue
	}

which errors if the interval is 0

metrictank/schema/metric.go

Lines 43 to 49 in b78a563

	func (m *MetricData) Validate() error {
	if m.OrgId == 0 {
	return ErrInvalidOrgIdzero
	}
	if m.Interval == 0 {
	return ErrInvalidIntervalzero
	}

I stripped out the publishing autointerval stuff in 5ac71f7 and made it error out if the Interval or ID is 0, and added comments to the interface making that explicit in 6c06422