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

Elasticsearch sink does not recover from failure to create index template #453

Closed
tnicolaysen opened this issue May 24, 2015 · 6 comments
Closed

Comments

@tnicolaysen
Copy link

(This issue spawned from this pull-request: serilog-contrib/serilog-sinks-elasticsearch#5)

Problem:
When a template is created (code), it can fail. If it fails, all successive indices created will not get the intended template settings.

Thoughts:
My biggest concern with not "self healing" it is that it will not be fixed before the app is restarted or app pool is recycled. It leaves the log configuration in a broken state as long as the application is running. New indexes created by Serilog will not have the template's setting.

So what's the consequence of not having a template? I'm not sure if it's serious.
For starters, you are going to have one or more suboptimal indices.
The way the template is set up right now, the issues might be that the user miss entries when searching or run into performance issues (in high-volume scenarios).
Should we try to mitigate some of these issues?

Another thought is that if template creation fails, something is most likely wrong with ES or the connection (assumption! @Mpdreamz, can you verify this?). That means that new events will most likely not be persisted. I hope the buffering feature kicks in here if it's activated.

Idea:
Self-healing might be desirable, but difficult to achieve in practice. One way would be to keep state on if the template exist and then retry until it is created in EmitBatch(...).
This mitigates the problem that the problem is not fixed before a restart.

Another way to deal with this is to have a way of configuring how to deal with template registering (and maybe other aspects of ES as well).
Maybe the code below could be a way of dealing with it. This means that the sink can have some sane defaults for the common user and leave it up to expert users to tweak it as needed.

new ElasticsearchSinkOptions
{
   ModifyTemplateSettings = ts => { 
      ts.EnsureTemplateExistBeforeWritingEvents = true;
      ts.StopEventsOnTemplateFailure = true;
   }
}
@mivano
Copy link
Member

mivano commented May 24, 2015

Would indeed be nice to add a new config for this to not break existing stuff and allow for more fine grained configuration like determining what the user wants to do when the template is not correctly applied. In theory it only needs to be there once but it is annoying when it is not there while you start writing data to an index.

@Mpdreamz
Copy link
Member

Register template fails could happen if the application is started before elasticsearch is, in most cases I suspect this only to happen on dev/dynamic test environments. But it could very well be that the application is started during a rolling or full cluster upgrade of elasticsearch where its temporarily unavailable.

  1. Register template fails, we start indexing anyway
  2. Register fails, we retry before each bulk otherwise we halt indexing relying on buffered output
  3. Register template fails, we index into a an index using a new DeadLetterIndexFormat option on ElasticsearchSinkOptions. default "deadletter-{0:yyyy.MM.dd}" ?

This could then be modeled and passed again on the sink options

RegisterTemplateFailure = 
    RegisterTemplateRecovery.[IndexAnyway | BufferUntilSucces | IndexToDeadletterIndex ]

Thoughts?

@mivano
Copy link
Member

mivano commented May 26, 2015

Looks like a nice approach to me. As long as we keep the default option as is (so IndexAnyway) we also do not break anything.

@drusellers
Copy link

I've been running into this lately with DEVS who just want to see the console out logs without having ES running. Big +1 from me for a fix here.

@drusellers
Copy link

In general this will bomb when the OnTick fires

            Serilog.LoggerConfiguration cfg = new LoggerConfiguration()
                .WriteTo.Elasticsearch(new ElasticsearchSinkOptions(new Uri("http://localhost:9200"))
                {
                    AutoRegisterTemplate = true,
                    BufferBaseFilename = @"C:\temp\elk",
                    BufferLogShippingInterval = TimeSpan.FromSeconds(5),
                    MinimumLogEventLevel = LogEventLevel.Verbose
                });

            cfg.CreateLogger().Information("HI");
            Console.ReadKey(true);

Assuming the server isn't running.

@nblumhardt
Copy link
Member

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

No branches or pull requests

6 participants