Skip to content
This repository has been archived by the owner on Jun 1, 2024. It is now read-only.

Add options to provide a custom template and overwrite an existing #51

Merged
merged 4 commits into from
Jul 16, 2016

Conversation

blachniet
Copy link
Contributor

These changes allow the user to supply a custom template and overwrite the template if it already exists in Elasticsearch.

GetTemplateContent can be set to return the body of the PUT template HTTP request. This allows users to further customize the Elasticsearch template, instead of using the standard Serilog template.

If OverwriteTemplate is set to true, the state class will not check to see if the template already exists before PUT-ing the new one, which will overwrite the template if it already exists.

@blachniet
Copy link
Contributor Author

Anything I can do to help get these changes, or something like them, integrated in? I've hacked around with my project to get this sort of functionality in, but I think it would make sense to have it in the sink. Are there oppositions to the changes I made, or something that I just screwed up on? Thanks!

@nblumhardt
Copy link
Contributor

Hi @blachniet,

I'm not an active maintainer of this sink but I notice your initial PR commit was non-mergeable and so CI failed. Merging upstream/dev back into your PR branch should kick off a new build.

Hope this helps,
Nick

@mivano
Copy link
Contributor

mivano commented Jul 11, 2016

Cool! Can you also add some details to the readme.md file?

/// When using the <see cref="AutoRegisterTemplate"/> feature, this allows you to override the default template content.
/// If not provided, a default template that is optimized to deal with Serilog events is used.
/// </summary>
public Func<string> GetTemplateContent { get; set; }
Copy link
Contributor

Choose a reason for hiding this comment

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

It might be more useful to return object here, the implementer could return a string or anonymous object in that case.

# Conflicts:
#	src/Serilog.Sinks.Elasticsearch/Sinks/ElasticSearch/ElasticsearchSinkState.cs
#	test/Serilog.Sinks.Elasticsearch.Tests/Serilog.Sinks.Elasticsearch.Tests.csproj
@blachniet
Copy link
Contributor Author

I made some of the suggested changes:

  • Merged in latest dev branch
  • Changed GetTemplateContent to return an object instead of string
  • Mentioned ability to define a custom template mapping in the README

I noticed that most of the code examples and feature details are described in wiki pages as opposed to the README. Should I add a code example there, with a link to it from the README, after it's merged in? Or should I add all that to the README?

Thanks!

@mivano mivano merged commit 2b58eda into serilog-contrib:dev Jul 16, 2016
@mivano
Copy link
Contributor

mivano commented Jul 16, 2016

Thanks @blachniet for the PR! I merged it now to dev and will create a merge to master soon to create new packages.

Regarding the wiki; I notice that I rather maintain the readme file then the wiki but they need to be in sync somehow. I will update it when it is merged to master.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants